Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add providing location for fetch_via_{vcs,git} #54

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aalexanderr
Copy link
Contributor

@aalexanderr aalexanderr commented Jun 10, 2021

This allows to call fetch_via_{vcs,git} multiple times for a location to
have another revision.

Example:

fetch_via_git("git+https://github.com/nexB/fetchcode.git", location="/tmp/repo")

will checkout tip of default branch

fetch_via_git("git+https://github.com/nexB/fetchcode.git@ccb7b6199681910ccf047f1a18aa89ece45d665c",
     location="/tmp/repo")

will reset to ccb7b61

Additionally:

  • removed some trailing whitespaces
  • used psf/black for code formatting (latest release, default values, let me know if I should've configured it not to switch ''->"" or sth other or if those commits should be dropped at all.
  • parametrize test functions to lessen code repetition

@TG1999
Copy link
Member

TG1999 commented Jun 10, 2021

Hey, @aalexanderr thanks for your contribution, please can you also add some tests ? @pombredanne please can you chime in :)

@TG1999
Copy link
Member

TG1999 commented Jun 10, 2021

And please consider using black for formatting of code, https://github.com/psf/black

This allows to call fetch_via_{vcs,git} multiple times for a location to
have another revision.

Example:
fetch_via_git("git+https://github.com/nexB/fetchcode.git", location="/tmp/repo")
will checkout tip of default branch
fetch_via_git("git+https://github.com/nexB/fetchcode.git@ccb7b6199681910ccf047f1a18aa89ece45d665c",
     location="/tmp/repo")
will reset to ccb7b61

Additionally remove some trailing whitespaces and fix indentation of a
docstring.

Signed-off-by: Alexander Mazuruk <a.mazuruk@samsung.com>
By using pytest.mark.parametrize decorator.

Signed-off-by: Alexander Mazuruk <a.mazuruk@samsung.com>
As recommended by @TG1999

Signed-off-by: Alexander Mazuruk <a.mazuruk@samsung.com>
As recommended by @TG1999

Signed-off-by: Alexander Mazuruk <a.mazuruk@samsung.com>
@aalexanderr
Copy link
Contributor Author

aalexanderr commented Jun 10, 2021

Hi, glad to contribute!
I'll work on adding tests for this tomorrow/monday.

Will also need to handle some cases like:
fetch_via_git("git+https://github.com/nexB/fetchcode.git", location="/tmp/repo")
fetch_via_git("git+https://github.com/nexB/scancode-toolkit.git", location="/tmp/repo")

@pombredanne
Copy link
Member

@TG1999 do you know if there is a way to get a shallow clone too?

quepop added a commit to quepop/fetchcode that referenced this pull request Jun 11, 2021
extends nexB#54

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
@aalexanderr
Copy link
Contributor Author

Hi, wanted to update -
When I started writing tests for stuff from pip.vcs I thought I'll better fix some stuff first:

  • no attribution to pip's code authors
  • as some pip.vcs code was commented out & due to @pombredanne question about shallow clone I decided to not move to pip as git submodule/subtree but rather clean it up for specialization needed in fetchcode
  • requirements.txt are updated & contain versions of pkgs to be used but are not actually used
  • lots of completely unused code (that doesn't seem relevant to me from this pkg point of view - let me know if I removed too much)

I'll add some tests in the near future but as I want it to be able to cover a bit more than is in scope of this PR I'll need a bit more time to work on it (as in mocks, fixtures etc)

As for this PR I'll probably split it too:

  • applying formatting on all fetchcode via black
  • vcs test parametrization (fyi- some test functions had same names, after this PR is applied the test no increases due to that)
  • the actual change from the title

@aalexanderr
Copy link
Contributor Author

@TG1999 do you know if there is a way to get a shallow clone too?

we might add it but it would need to be properly documented as some of the features of shallow clone are not yet done in git
https://git-scm.com/docs/shallow
thus we might also warn users that e.g. getting tags from history in case of shallow clone might not be possible (yet) + possible other stuff from previous versions of git

quepop added a commit to quepop/fetchcode that referenced this pull request Jun 23, 2021
 -extends nexB#54
 -Added filename deduction (content-disposition/URL)
 -Fetch and its helper functions now use pathlib's Path

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
quepop added a commit to quepop/fetchcode that referenced this pull request Jun 23, 2021
 -extends nexB#54
 -Added filename deduction (content-disposition/URL)
 -Fetch and its helper functions now use pathlib's Path

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
quepop added a commit to quepop/fetchcode that referenced this pull request Jun 24, 2021
 -extends nexB#54
 -Added filename deduction (content-disposition/URL)
 -Fetch and its helper functions now use pathlib's Path

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
quepop added a commit to quepop/fetchcode that referenced this pull request Jun 24, 2021
 -extends nexB#54
 -Added filename deduction (content-disposition/URL)
 -Fetch and its helper functions now use pathlib's Path

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
quepop added a commit to quepop/fetchcode that referenced this pull request Jun 25, 2021
 -extends nexB#54
 -Added filename deduction (content-disposition/URL)
 -Fetch and its helper functions now use pathlib's Path

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
@aalexanderr
Copy link
Contributor Author

regarding shallow clone, this might be better (already implemented in pip's vcs) https://git-scm.com/docs/partial-clone

aalexanderr pushed a commit to aalexanderr/fetchcode that referenced this pull request Sep 2, 2021
 -extends nexB#54
 -Added filename deduction (content-disposition/URL)
 -Fetch and its helper functions now use pathlib's Path

Signed-off-by: Mateusz Perc <m.perc@samsung.com>
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.

None yet

3 participants