cli: add git-upgrade-shared-modules command #371

mvidalgarcia · 2020-08-06T13:44:19Z

closes #365

TODO

Update docstrings
Find a better/different name for old git-upgrade-shared-modules (the one that creates PRs)
Verify that both work as expected

reana/reana_dev/utils.py

diegodelemos · 2020-08-07T14:05:52Z

reana/reana_dev/utils.py

+    return get_current_branch(get_srcdir(component)) != "master"
+
+
+def replace_version_string(


Thanks, looks great! 😊 This covers all the use cases in the create release commit. Just two observations:

We could rename the function since this replaces strings in component files, no matter what they are

What about renaming module_grep to something along the lines of: line_selector_expression/line_regex/line_pattern (not very gifted today with the naming but you know what I mean)

Mmh what about replace_string, replace_regex, sed_file...?

Naming on these tricky functions is always hard, maybe line_selector_regex?

Updated, feel free to change it if needed.

mvidalgarcia · 2020-08-07T15:35:45Z

reana/reana_dev/git.py

+                "git branch --show-current", component, return_output=True
+            )
+            run_command(
+                f"git push --force origin {branch}", component,


I'm not fully sure about this part. If one is checking out a feature branch of a colleague, will this upgrade their PR?

Nope, if you are checking only, you are in "latest" or "debug" mode, so you wouldn't need to upgrade minimal versions. The command to upgrade common modules is only used by the integrator, where it is OK to update the PR, because it saves time since the integrator won't need to ask people "please bump r-commons in the other PR in this set". So I consider it as a "service provided by integrators so that developers don't have to do unnecessary bumping".

tiborsimko · 2020-08-11T08:29:03Z

Find a better/different name for old git-upgrade-shared-modules (the one that creates PRs)

I would simply remove the old function, because the new functions can take over; a workflow like:

$ reana-dev git-create-release-commit -c CLUSTER -c CLIENT
$ reana-dev git-upgrade-shared-modules -c CLUSTER -c CLIENT
$ reana-dev git-create-pr -c CLUSTER -c CLIENT

If some functionality is missing, e.g. force creation of a release commit in order to pass from 0.7.0aN to 0.7.0, then we can always add this as new options to the commands, say --force-new-tag 0.7.0 and such.

tiborsimko · 2020-08-11T08:30:22Z

reana/reana_dev/utils.py

+            line_selector_regex=f"{module}.*>=",
+            component=component,
+        )
+        replace_version_string(


I get troubles with this branch:

$ reana-dev git-upgrade-shared-modules -c r-w-controller ... File "/home/simko/.virtualenvs/reana/lib/python3.8/site-packages/click/decorators.py", line 21, in new_func return f(get_current_context(), *args, **kwargs) File "/home/simko/.virtualenvs/reana/lib/python3.8/site-packages/reana/reana_dev/git.py", line 817, in git_upgrade_shared_modules update_module_in_cluster_components( File "/home/simko/.virtualenvs/reana/lib/python3.8/site-packages/reana/reana_dev/utils.py", line 437, in update_module_in_cluster_components replace_version_string( NameError: name 'replace_version_string' is not defined

tiborsimko · 2020-08-11T08:59:58Z

reana/reana_dev/git.py

-        )
+    _create_commit_or_amend(components)
+    ctx.invoke(git_diff, component=component)
+    if click.confirm("Do you want to push your feature branches?"):


Instead of having an interactive question, what about adding a new --push CLI option, similarly to the git-merge command, and only push when --push option is passed? (By default, it would not be.) Advantage: consistent behaviour of several git-... commands improves UX.

tiborsimko · 2020-08-11T09:12:41Z

reana/reana_dev/utils.py

+            component=component,
+        )
+        replace_string(
+            file_="requirements.txt",


Updating requirements.txt should be optional, because e.g. r-client does not contain it.

Currently it gives an error:

$ reana-dev git-upgrade-shared-modules -c r-client --create-new-version-bump-commit ... [2020-08-11T11:10:33] : cat requirements.txt | grep -n -e "reana-commons.*==" | cut -f1 -d: cat: requirements.txt: No such file or directory

Expected behaviour is: if requirements.txt file exists, update it; otherwise continue.

tiborsimko · 2020-08-11T09:16:33Z

reana/reana_dev/git.py

+    " shared modules points at a tag, in other case, the command will be aborted.",
+)
+@click.option(
+    "--create-new-version-bump-commit",


Upon second thought, what about inverting the logic and creating new version bump commit by default unless --amend is passed? Advantage: it would behave just like git, so less surprises for people.

If we like it, then:

let's rename --create-new-version-bump-commit to --amend

let's make it false by default

let's invert the implementation to always make release commits unless --amend is used.

WDYT?

closes reanahub#365

tiborsimko · 2020-08-11T11:49:33Z

reana/reana_dev/git.py

-    " If so, a commit and a PR with"
-    ' "installation: shared packages version bump" message will'
-    " be created for all affected CLUSTER components.",
+    help="Should the changes be integrated as part of the latest commit of each component?.",


End phrase with ? instead of ?.

tiborsimko · 2020-08-11T11:49:40Z

reana/reana_dev/git.py

+    "--push",
+    is_flag=True,
+    default=False,
+    help="Should the feature branches with the upgrades be pushed?.",


End phrase with ? instead of ?.

... also, "be pushed to origin?" because elsewhere in other r-dev commands we are pushing to both origin/upstream... so it may help to be more explicit here.

tiborsimko

All good, left final 2-3 cosmetic comments for the help strings

mvidalgarcia force-pushed the 365-git-upgrade-common-modules branch 2 times, most recently from 6bc5a28 to e078cdd Compare August 6, 2020 17:32

mvidalgarcia changed the title ~~cli: add git-upgrade-common-modules command~~ cli: add git-upgrade-shared-modules command Aug 6, 2020

mvidalgarcia force-pushed the 365-git-upgrade-common-modules branch 2 times, most recently from 82c518b to c06163b Compare August 7, 2020 10:34

mvidalgarcia marked this pull request as ready for review August 7, 2020 10:36

diegodelemos reviewed Aug 7, 2020

View reviewed changes

reana/reana_dev/utils.py Outdated Show resolved Hide resolved

diegodelemos reviewed Aug 7, 2020

View reviewed changes

reana/reana_dev/utils.py Outdated Show resolved Hide resolved

diegodelemos reviewed Aug 7, 2020

View reviewed changes

reana/reana_dev/utils.py Outdated Show resolved Hide resolved

mvidalgarcia force-pushed the 365-git-upgrade-common-modules branch 2 times, most recently from 98b4405 to 0458704 Compare August 7, 2020 13:42

diegodelemos reviewed Aug 7, 2020

View reviewed changes

mvidalgarcia force-pushed the 365-git-upgrade-common-modules branch from 0458704 to c42e205 Compare August 7, 2020 14:55

mvidalgarcia commented Aug 7, 2020

View reviewed changes

tiborsimko reviewed Aug 11, 2020

View reviewed changes

diegodelemos force-pushed the 365-git-upgrade-common-modules branch from c42e205 to ef7c49d Compare August 11, 2020 08:52

tiborsimko reviewed Aug 11, 2020

View reviewed changes

diegodelemos force-pushed the 365-git-upgrade-common-modules branch from ef7c49d to 2299fde Compare August 11, 2020 09:47

cli: add git-upgrade-shared-modules command

f8e2558

closes reanahub#365

diegodelemos force-pushed the 365-git-upgrade-common-modules branch from 2299fde to 45fe7a6 Compare August 11, 2020 09:53

tiborsimko reviewed Aug 11, 2020

View reviewed changes

tiborsimko approved these changes Aug 11, 2020

View reviewed changes

cli: remove git_upgrade_all_shared_modules

ccb00ee

diegodelemos force-pushed the 365-git-upgrade-common-modules branch from 45fe7a6 to ccb00ee Compare August 11, 2020 15:26

diegodelemos merged commit ccb00ee into reanahub:master Aug 11, 2020

mvidalgarcia deleted the 365-git-upgrade-common-modules branch August 15, 2020 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add git-upgrade-shared-modules command #371

cli: add git-upgrade-shared-modules command #371

mvidalgarcia commented Aug 6, 2020 •

edited by tiborsimko

diegodelemos Aug 7, 2020

mvidalgarcia Aug 7, 2020

mvidalgarcia Aug 7, 2020

mvidalgarcia Aug 7, 2020

tiborsimko Aug 11, 2020

tiborsimko commented Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko Aug 11, 2020

tiborsimko left a comment

		return get_current_branch(get_srcdir(component)) != "master"


		def replace_version_string(

cli: add git-upgrade-shared-modules command #371

cli: add git-upgrade-shared-modules command #371

Conversation

mvidalgarcia commented Aug 6, 2020 • edited by tiborsimko

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborsimko commented Aug 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborsimko left a comment

Choose a reason for hiding this comment

mvidalgarcia commented Aug 6, 2020 •

edited by tiborsimko