Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

BF: remove a submodule with a remote without refs + misc fixes around#521

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

yarikoptic
Copy link
Contributor

now if there is a remote without any refs (this happens e.g. with git-annex repo with special remotes), it would refuse to remove the submodule
(with 2.0.8 it also didn't work but with a different msg "AssertionError: Remote datalad did not have any references")

@ankostis
Copy link
Contributor

@yarikoptic would it be possible to add a test-case into the PR with the annex command, replicating the situation you just described?

@yarikoptic
Copy link
ContributorAuthor

ok@ankostis -- but along the way had to fix 2 other bugs... apparently some tests (which used@with_rw_directory) were not run at all! so you can expect failures of those tests which were effected


parent.index.commit("Added submodule")

assert sm.repo is parent # yoh was surprised since expected sm repo!!
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

and I am not sure why that is the case -- I was expecting submodules repo to be the repo of the submodule ... but I guess it can make sense to refer to parent which contains the submodule, but then is there an attribute for submodules repo (if present)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@Byron any comment? I'm not familiar with git-modules.

@yarikoptic
Copy link
ContributorAuthor

reincarnating@with_rw_directory lead to one failing test

======================================================================FAIL: test_lock_reentry (git.test.test_config.TestBase)----------------------------------------------------------------------Traceback (most recent call last):  File "/home/travis/build/gitpython-developers/GitPython/git/test/lib/helper.py", line 95, in wrapper    return func(self, path)  File "/home/travis/build/gitpython-developers/GitPython/git/test/test_config.py", line 100, in test_lock_reentry    self.failUnlessRaises(IOError, GitConfigParser, fpl, read_only=False)AssertionError: IOError not raised-------------------- >> begin captured logging << --------------------git.util: INFO: Test TestBase.test_lock_reentry failed, output is at '/tmp/test_lock_reentryfSovNi'

I am not sure on state of locking/context manager expectations atm and see recent work touching locking... so I will need to leave this to you guys (@ankostis ?) to figure out

@yarikopticyarikoptic changed the titleBF: Allow to remove a submodule with a remote without refsBF: remove a submodule with a remote without refs + misc fixes aroundOct 2, 2016
yarikoptic added a commit to yarikoptic/GitPython that referenced this pull requestOct 2, 2016
codecov in our (datalad, etc) experience provides a better service,great support, and super-nice intergration with chromium and firefoxfor reviewing coverage of pull requests.  In light of the@with_rw_directory fiasco detected/fixed ingitpython-developers#521 I would stronglyrecommend to (re-)enable and use coverage reports
yarikoptic added a commit to yarikoptic/GitPython that referenced this pull requestOct 2, 2016
codecov in our (datalad, etc) experience provides a better service,great support, and super-nice intergration with chromium and firefoxfor reviewing coverage of pull requests.  In light of the@with_rw_directory fiasco detected/fixed ingitpython-developers#521 I would stronglyrecommend to (re-)enable and use coverage reports
@ankostis
Copy link
Contributor

@yarikoptic so you mean thatgit.test.test_config:TestBase.test_lock_reentry() that is failing is included in the CI for the first time?

@yarikoptic
Copy link
ContributorAuthor

Yes, at least since some time in the past

@yarikoptic
Copy link
ContributorAuthor

If your really want to know, I could check later since when

@ankostis
Copy link
Contributor

ankostis commentedOct 2, 2016•
edited
Loading

Indeed@yarikoptic your31fd955 fixed an important regression introduced by#519,
where I had practically disabled 19 TCs!
(so the ALL GREEN was an illusion :-/, check#519 for the actual list of TC errors on Windows)

So, I'm cherry picking your2528d11 and31fd955 but modified to include only yourhelper.py fixes and pushing them into myown repo, to try to fix the failing TC; for thetest_submodles.py changes, they have to wait, and I will collect them and re-synthesize a new branch to merge.

@ankostisankostis merged commitf284a4e intogitpython-developers:masterOct 2, 2016
ankostis added a commit that referenced this pull requestOct 2, 2016
+ The actual commits have been re-written and rebased previously.
@ankostis
Copy link
Contributor

On Travis, PY3.3 failed due to file-handles; increased ulimit 96-->100 solved the problem.
On Appveyor, this is a drama...

@ankostis
Copy link
Contributor

ankostis commentedOct 2, 2016•
edited
Loading

Forgot to thank you.
@yarikoptic If you may, add yourself on the Authors, on this branch, and I will cherry-pick it, just add a comment here to notify me.

@yarikoptic
Copy link
ContributorAuthor

That is OK, if you like to see me there - add, otherwise I am just fine. Cheers. Thanks for attending to my PRs quickly

@yarikopticyarikoptic deleted the bf-rsubmodule-remove branchOctober 15, 2016 05:27
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ankostisankostisankostis left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@yarikoptic@ankostis
[8]ページ先頭
©2009-2025 Movatter.jp