Skip to content

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jul 26, 2022

This is based on #55 by @mohd-akram

But I am actually not convinced, that it does what it should.

Why do we go recursively till to the root when searching for the git folder?! I actually would expect to drill down. But still, it means, that if somebody has a .git-folder in his root folder, that we will process it. Smells very fishy.

I changed the behaviour slightly: If the found path is not a folder, it will return null and not the path.

Checklist

@Uzlopak Uzlopak requested review from jsumners and climba03003 July 26, 2022 10:37
@Uzlopak Uzlopak linked an issue Jul 26, 2022 that may be closed by this pull request
@Uzlopak Uzlopak changed the title implement submodules handling Support git-submodules Jul 26, 2022
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 26, 2022

@jsumners

better?

@jsumners
Copy link
Member

Which tests are validating the git-submodule functionality?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 26, 2022

That is the question I actually ask in my first comment. I wrote the unit tests and it does behave now like @mohd-akram s code change. But the question is, does it really fix it? I have actually no clue, as I personally dont use submodules.

And how should I check in an integration test into git/github, when I have to basically have to patch .git folder etc..

@jsumners
Copy link
Member

That is the question I actually ask in my first comment. I wrote the unit tests and it does behave now like @mohd-akram s code change. But the question is, does it really fix it? I have actually no clue, as I personally dont use submodules.

And how should I check in an integration test into git/github, when I have to basically have to patch .git folder etc..

I think the only real way to test this whole module is through integration tests. Those would be done via a Docker container. The steps would be something like:

  1. Launch container with a mount to a stub project, e.g. a integration/git-submodule/ directory in this project
  2. Do an npm install in that project
  3. Verify the .git/hooks/pre-commit hook is correct
  4. Verify submodules are installed correctly (or whatever)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support git-submodules
2 participants