-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add temporary method for disabling shallow cloning (#5031) #5036
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
Conversation
This adds a project feature that allows us to use a standard clone and fetch rather than the shallow clone/fetch introduced in #4939. Eventually we should move this to the web UI, but doing so requires some work to make sure, for example, that git options are only show when 'Project.repo_type' is 'git'. Signed-off-by: Stephen Finucane <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks.
def test_use_shallow_clone(self): | ||
repo = self.project.vcs_repo() | ||
repo.update() | ||
repo.checkout('submodule') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to call .clone
(mocked) here and check what were the arguments used to call it. That way, we are sure that we are using use_shallow_clone
and it does have an effect in the flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we go for using a feature flag, this looks good for me
|
||
.. note:: | ||
|
||
Temporarily, we support skipping this option as builds that rely on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can remove this note if we go for using a feature flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking was to include this for now so we can unblock the broken builds then decide at a later date whether we wanted to expose this to users. If we don't, we remove the note. If we do, we remove all of this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I figured we'd need something like this, so glad to see it contributed, thanks!
Going to hotfix this in a deploy today to unblock you. 👍 We should also plan to document our user-impacting feature flags, so people know they can ask us to turn them on :) |
@stephenfin This should now be deployed. Let me know if it is working 👍 |
This adds a project feature that allows us to use a standard clone and
fetch rather than the shallow clone/fetch introduced in #4939.
Eventually we should move this to the web UI, but doing so requires some
work to make sure, for example, that git options are only show when
'Project.repo_type' is 'git'.
Closes #5031