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

New actions #14

Merged
merged 2 commits into from
Sep 9, 2015
Merged

New actions #14

merged 2 commits into from
Sep 9, 2015

Conversation

chintal
Copy link
Contributor

@chintal chintal commented Sep 9, 2015

Added actions in and rev.

in

(previously cu)

Check for incoming changes before applying them.

Typical usage is to determine which repositories have incoming updates, and to use that information to prevent expected problems by cleaning up necessary trees when up is run, or by not running up at that point if it's likely to cause merge conflicts or the like. in should not change the local filesystem in any way (working copy or repository), given that we don't make any assumptions about the workflow the user wants to use.

rev

Print the current revision of the working tree. There isn't much use for it when run as a script, but there are use cases for being able to obtain this revision information when used as a module. My immediate use case is to include the revision information of semi-automated test definitions when generating test reports from semi-automated or automated tests. I'm using pysvn to get the revision information for the moment, which works fine, but locks me in to svn.

Implementations notes

Both of these work actions work fine for SVN, and if there are problems or corner cases I'll see them soon enough.

bzr, git, and hg implementations have only been tested on simple repositories with a simple upstream and clean working trees. I think they should still work, but some care may be called for before making use of these actions in production for those version systems.

output = system("bzr missing")
output = output.strip()
output_lines = output.split('\n')
if "Branches are up to date." in output_lines:
Copy link
Owner

Choose a reason for hiding this comment

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

Lines like this are very brittle. I'm speaking from experience with zest.releaser here. Problems I've seen:

  • Suddenly the initial letter 'B' becomes a 'b' in a newer version of bzr (or one of the other ones).
  • Someone has a localized shell and bzr nicely renders the message in french/german/dutch/whatever...

I'm not against testing for the string output like you're doing: there's not much else we can do. I've tried looking at exit codes in zest.releaser and that was even worse :-) There are two things we can do:

  • At least do a .lower() before comparing.
  • Add a short note in the readme for the "cu" command that it is inherently a bit fragile as it depends on the english textual output of the various commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I've tried to use the most apparently reliable indicators for each command, but my experience with both bzr and hg was some years ago.

Since these commands are alongside the checkoutmanager functionality that exists at present and don't interfere with it otherwise, I propose starting out with something which seems to be acceptable and tweak it as and when issues are discovered.

If there is a more reliable indicator, I'm more than happy to switch to that. For example, I'm currently considering bzr revno for up instead of bzr log -r-1. This should more reliably return the revision number.

Copy link
Owner

Choose a reason for hiding this comment

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

I agree that this new command is separate from the existing ones. So something that's mostly working is fine and we can tweak it later when needed.

So what you have now is fine (apart from adding a .lower()), though you are free to experiment with that "bzr log -r -1" stuff of course :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

For bzr I think bzr missing --theirs-only is better.

@reinout
Copy link
Owner

reinout commented Sep 9, 2015

@mauritsvanrees, can you take a quick look?

@reinout
Copy link
Owner

reinout commented Sep 9, 2015

@chintal : can you give a ping when you consider the PR to be ready? Then I can have a final look and merge it.


@capture_stdout
def cmd_cu(self):
output = system("git pull --dry-run")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe git fetch? That already gets the code, without updating the working copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cu/in shouldn't be doing anything that changes the local filesystem

@mauritsvanrees
Copy link
Collaborator

I added some comments. Seems okay otherwise.

Idea: maybe add LANG=C in front of all the commands to get rid of language differences?

@reinout
Copy link
Owner

reinout commented Sep 9, 2015

LANG=C? Oh wow, if that works it would be nice! It cannot hurt to test.

"Does it work on windows" is the number one question that springs to my suspicious mind :-)

@chintal
Copy link
Contributor Author

chintal commented Sep 9, 2015

@reinout I think it's ready to go.

@@ -71,6 +71,15 @@ co
missing
Print directories that are missing from the config file

out
Copy link
Owner

Choose a reason for hiding this comment

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

Weird! So "out" wasn't documented yet? Good catch :-)

reinout added a commit that referenced this pull request Sep 9, 2015
@reinout reinout merged commit d8e5674 into reinout:master Sep 9, 2015
@reinout
Copy link
Owner

reinout commented Sep 9, 2015

Thanks for your work! I've released 2.4.

Can you check if it works? And review the changelog? https://github.com/reinout/checkoutmanager/blob/master/CHANGES.rst#24-2015-09-09

@chintal chintal deleted the new_actions branch September 10, 2015 06:51
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.

3 participants