Skip to content

ENH: Add a script to update the remote module commit hashes#632

Merged
jhlegarreta merged 1 commit intoInsightSoftwareConsortium:masterfrom
jhlegarreta:AddRemotesHashUpdateScript
Mar 28, 2019
Merged

ENH: Add a script to update the remote module commit hashes#632
jhlegarreta merged 1 commit intoInsightSoftwareConsortium:masterfrom
jhlegarreta:AddRemotesHashUpdateScript

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

@jhlegarreta jhlegarreta commented Mar 25, 2019 •
edited
Loading

Add a script to update the remote module commit hashes.

Update the Release.md guide to add the remote module update step prior
to a release.

@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Mar 25, 2019 •
edited
Loading

As discussed some time ago by mail.

A few comments:

  • This will surely need further testing.
  • jq must be available.
  • Have not tested if it would fail if for some reason the remote repository URL does not have a trailing .git
  • May be the coding style is not too bash-like, so feel free to suggest changes.
  • We may need to ask for the username so that the user is authenticated in the curl command. I think there is a limit in the anonymous gets.
  • I guess generating a branch and automatically committing/pushing as @hjmjohnson has done in some scripts is also doable, but this is a first version.

@jhlegarreta jhlegarreta added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Remotes Issues affecting the Remote module labels Mar 25, 2019
thewtex
thewtex requested changes Mar 26, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Super cool @jhlegarreta! 🥇

A few suggestions inline.

Comment thread Utilities/Maintenance/UpdateRemoteModules.sh Outdated
Comment thread Utilities/Maintenance/UpdateRemoteModules.sh
@thewtex
Copy link
Copy Markdown
Member

thewtex commented Mar 26, 2019

This also provides a dependency-free way to get the latest commit on GitHub:

https://stackoverflow.com/questions/15677439/command-to-get-latest-git-commit-hash-from-a-branch

dzenanz
dzenanz reviewed Mar 26, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Excellent that you got to this! Matt has already provided excellent feedback.

@jhlegarreta jhlegarreta force-pushed the AddRemotesHashUpdateScript branch 2 times, most recently from b68921c to e9d1318 Compare March 27, 2019 02:32
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Mar 27, 2019

This also provides a dependency-free way to get the latest commit on GitHub:

https://stackoverflow.com/questions/15677439/command-to-get-latest-git-commit-hash-from-a-branch

Nice ! Done in e9d1318 and a variable name error fixed !

Thanks both.

@jhlegarreta jhlegarreta force-pushed the AddRemotesHashUpdateScript branch from e9d1318 to 509207d Compare March 27, 2019 03:36
dzenanz
dzenanz approved these changes Mar 27, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Looks good!

@jhlegarreta
ENH: Add a script to update the remote module commit hashes
8fdb454 
Add a script to update the remote module commit hashes.

Update the `Release.md` guide to add the remote module update step prior
to a release.
@jhlegarreta jhlegarreta force-pushed the AddRemotesHashUpdateScript branch from 509207d to 8fdb454 Compare March 28, 2019 00:15
jhlegarreta
jhlegarreta commented Mar 28, 2019
View reviewed changes
Comment thread Documentation/Maintenance/Release.md

1. Update the ITK tag used in the `azure-pipelines.yml` CI configuration and the
`setup.py` Python setup files using the [](https://github.com/InsightSoftwareConsortium/ITK/tree/master/Utilities/Maintenance/)
script. This will involve merging a new pull request to each remote module
Copy link
Copy Markdown
Member Author

@jhlegarreta jhlegarreta Mar 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Used the term involve because the commit/PR is not yet automatic.

Eventually a Documentation/Maintenance/UpdatingRemotes.md could be created, much like it exists for the ThirdParty modules, and that document could be linked from here. But that is left for an eventual future PR.

@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Mar 28, 2019

Added a note in the Release.md document concerning the update of the Remote modules.

thewtex
thewtex approved these changes Mar 28, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Thanks @jhlegarreta ! This will be very helpful.

@phcerdan
Copy link
Copy Markdown
Contributor

phcerdan commented Mar 28, 2019

This is great @jhlegarreta!

@jhlegarreta jhlegarreta merged commit 63a7c3a into InsightSoftwareConsortium:master Mar 28, 2019
@jhlegarreta jhlegarreta deleted the AddRemotesHashUpdateScript branch March 28, 2019 03:16
@jhlegarreta jhlegarreta mentioned this pull request Mar 30, 2019
Merged
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@jhlegarreta
Merge pull request InsightSoftwareConsortium#632 from jhlegarreta/Add…
6abae0c 
…RemotesHashUpdateScript

ENH: Add a script to update the remote module commit hashes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thewtex thewtex thewtex approved these changes

@dzenanz dzenanz dzenanz approved these changes

@phcerdan phcerdan phcerdan approved these changes

@jcfr jcfr Awaiting requested review from jcfr

@hjmjohnson hjmjohnson Awaiting requested review from hjmjohnson

@blowekamp blowekamp Awaiting requested review from blowekamp

@fbudin69500 fbudin69500 Awaiting requested review from fbudin69500

Assignees

No one assigned

Labels

area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jhlegarreta @thewtex @phcerdan @dzenanz