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

Implement submodules key from v2 config #4493

Merged
merged 19 commits into from Oct 2, 2018

Conversation

Projects
None yet
4 participants
@stsewd
Member

stsewd commented Aug 8, 2018

This PR on top of #4456. It should be merged after. New changes are from 2fee268

Closes #4464

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Aug 9, 2018

Member

The only thing missing is to test v1 with submodules, but I kind of need what I have in #4394

Member

stsewd commented Aug 9, 2018

The only thing missing is to test v1 with submodules, but I kind of need what I have in #4394

@humitos

This comment has been minimized.

Show comment
Hide comment
@humitos

humitos Aug 15, 2018

Member

I will wait until the base PR gets merged to review this.

Member

humitos commented Aug 15, 2018

I will wait until the base PR gets merged to review this.

@agjohnson

This comment has been minimized.

Show comment
Hide comment
@agjohnson

agjohnson Aug 27, 2018

Contributor

This should be ready for rebase. It's already set to master base.

Contributor

agjohnson commented Aug 27, 2018

This should be ready for rebase. It's already set to master base.

@agjohnson agjohnson added this to the YAML File Completion milestone Aug 27, 2018

return False
# Keep compatibility with previous projects
code, out, _ = self.run('git', 'submodule', 'status', record=False)

This comment has been minimized.

@stsewd

stsewd Aug 30, 2018

Member

We were running this code to check for submodules previously, so we need to keep the same behavior here

@stsewd

stsewd Aug 30, 2018

Member

We were running this code to check for submodules previously, so we need to keep the same behavior here

@@ -43,6 +43,7 @@ class BaseVCS(object):
supports_tags = False # Whether this VCS supports tags or not.
supports_branches = False # Whether this VCS supports branches or not.
supports_submodules = False

This comment has been minimized.

@stsewd

stsewd Aug 30, 2018

Member

This way we can say what vcs needs to do an extra step

@stsewd

stsewd Aug 30, 2018

Member

This way we can say what vcs needs to do an extra step

@agjohnson

This comment has been minimized.

Show comment
Hide comment
@agjohnson

agjohnson Sep 7, 2018

Contributor

Is this still WIP? Just needs a rebase if not

Contributor

agjohnson commented Sep 7, 2018

Is this still WIP? Just needs a rebase if not

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Sep 7, 2018

Member

I only have a doubt in #4493 (comment), and then test this for real with the sphinx or mkdocs implementation

Member

stsewd commented Sep 7, 2018

I only have a doubt in #4493 (comment), and then test this for real with the sphinx or mkdocs implementation

@stsewd stsewd self-assigned this Sep 12, 2018

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Sep 28, 2018

Member

I figure out that we don't need the lock in the submodules operation, I guess the lock is there so we don't end with several builds of the same version. This PR is tested and ready for review.

Member

stsewd commented Sep 28, 2018

I figure out that we don't need the lock in the submodules operation, I guess the lock is there so we don't end with several builds of the same version. This PR is tested and ready for review.

@agjohnson

This looks great! 👍

I can't think of any other changes required here, so i think this is just safe to merge.

@agjohnson agjohnson merged commit bfa6bdb into rtfd:master Oct 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stsewd stsewd deleted the stsewd:break-submodules-step branch Oct 2, 2018

@ax3l

This comment has been minimized.

Show comment
Hide comment
@ax3l

ax3l Oct 2, 2018

@agjohnson with this PR merged (thx a lot! :) ), is there anything we can add to our .readthedocs.yml already that will exclude recursive git submodule init?

ax3l commented Oct 2, 2018

@agjohnson with this PR merged (thx a lot! :) ), is there anything we can add to our .readthedocs.yml already that will exclude recursive git submodule init?

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Oct 2, 2018

Member

@ax3l this isn't deployed yet, and it's part of the v2 of our configuration file, I'm trying to get some users to a beta soon, please let your project here #1621 (comment)

Member

stsewd commented Oct 2, 2018

@ax3l this isn't deployed yet, and it's part of the v2 of our configuration file, I'm trying to get some users to a beta soon, please let your project here #1621 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment