FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

BF: remove a submodule with a remote without refs + misc fixes around by yarikoptic · Pull Request #521 · gitpython-developers/GitPython · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

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

Merged
ankostis merged 4 commits into
gitpython-developers:masterfrom
yarikoptic:bf-rsubmodule-remove
Oct 2, 2016
ConversationCommits (4)ChecksFiles changed
Merged

BF: remove a submodule with a remote without refs + misc fixes around#521
ankostis merged 4 commits into
gitpython-developers:masterfrom
yarikoptic:bf-rsubmodule-remove

Conversation

yarikoptic commented Oct 2, 2016

Copy link
Copy Markdown
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")

yarikoptic mentioned this pull request Oct 2, 2016
Closed

ankostis commented Oct 2, 2016

Copy link
Copy Markdown
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 commented Oct 2, 2016

Copy link
Copy Markdown
Contributor Author

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

yarikoptic commented Oct 2, 2016
View reviewed changes
Comment thread git/test/test_submodule.py
assert sm.exists()

parent.index.commit("Added submodule")

yarikoptic Oct 2, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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)?

ankostis Oct 2, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

yarikoptic commented Oct 2, 2016

Copy link
Copy Markdown
Contributor Author

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

yarikoptic changed the title BF: Allow to remove a submodule with a remote without refs BF: remove a submodule with a remote without refs + misc fixes around Oct 2, 2016
yarikoptic added a commit to yarikoptic/GitPython that referenced this pull request Oct 2, 2016
RF: coveralls (not used/relied on really) -> codeco
18caa95 
codecov in our (datalad, etc) experience provides a better service,
great support, and super-nice intergration with chromium and firefox
for reviewing coverage of pull requests.  In light of the
@with_rw_directory fiasco detected/fixed in gitpython-developers#521 I would strongly
recommend to (re-)enable and use coverage reports
yarikoptic mentioned this pull request Oct 2, 2016
Merged
yarikoptic added a commit to yarikoptic/GitPython that referenced this pull request Oct 2, 2016
RF: coveralls (not used/relied on really) -> codecov
94ca83c 
codecov in our (datalad, etc) experience provides a better service,
great support, and super-nice intergration with chromium and firefox
for reviewing coverage of pull requests.  In light of the
@with_rw_directory fiasco detected/fixed in gitpython-developers#521 I would strongly
recommend to (re-)enable and use coverage reports

ankostis commented Oct 2, 2016

Copy link
Copy Markdown
Contributor

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

yarikoptic commented Oct 2, 2016

Copy link
Copy Markdown
Contributor Author

Yes, at least since some time in the past

yarikoptic commented Oct 2, 2016

Copy link
Copy Markdown
Contributor Author

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

ankostis commented Oct 2, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Indeed @yarikoptic your 31fd955 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 your 2528d11 and 31fd955 but modified to include only your helper.py fixes and pushing them into my own repo, to try to fix the failing TC; for the test_submodles.py changes, they have to wait, and I will collect them and re-synthesize a new branch to merge.

ankostis mentioned this pull request Oct 2, 2016
Merged
ankostis merged commit f284a4e into gitpython-developers:master Oct 2, 2016
ankostis added a commit that referenced this pull request Oct 2, 2016
"Null" merge yarik/bf-rsubmodule-remove to close #521
3dc2f73 
+ The actual commits have been re-written and rebased previously.

ankostis commented Oct 2, 2016

Copy link
Copy Markdown
Contributor

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

ankostis commented Oct 2, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 commented Oct 3, 2016

Copy link
Copy Markdown
Contributor Author

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

yarikoptic deleted the bf-rsubmodule-remove branch October 15, 2016 05:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
ankostis ankostis left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL