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

Add "Produced by tinuous" message to Datalad commit messages #86

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Jun 10, 2021

Closes #82.

@jwodder jwodder added the minor Increment the minor version when merged label Jun 10, 2021
@@ -152,6 +152,7 @@ def fetch(cfg: Config, state: str, sanitize_secrets: bool) -> None:
msg += f", {artifacts_added} artifacts added"
if relassets_added:
msg += f", {relassets_added} release assets added"
msg += f"\n\nProduced by tinuous v{__version__}"
Copy link
Member

Choose a reason for hiding this comment

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

since we do not have v prefix in the tags or release versions, I do not see a need to prepend v here, why not to keep it consistent with output of tinuous --version and thus simply

Suggested change
msg += f"\n\nProduced by tinuous v{__version__}"
msg += f"\n\nProduced by tinuous {__version__}"

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just think "Produced by tinuous v0.3.0" looks better than "Produced by tinuous 0.3.0". No one's going to expect that the version here will be exactly the same as the tag name.

Copy link
Member

Choose a reason for hiding this comment

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

oh well, I don't care about looks enough I guess, and more about consistency (not necessarily to match the tags but overall, and matching to the tags just becomes a side-effect of that). Do you feel strong about the "looks"? ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I don't feel strongly, I was just stating my case. I'll get rid of the v.

@codecov-commenter
Copy link

Codecov Report

Merging #86 (eef43da) into master (dd48e39) will decrease coverage by 0.85%.
The diff coverage is 83.33%.

❗ Current head eef43da differs from pull request most recent head 0bb368d. Consider uploading reports for the commit 0bb368d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #86      +/-   ##
==========================================
- Coverage   65.67%   64.82%   -0.86%     
==========================================
  Files           9        9              
  Lines         810      813       +3     
  Branches      120      120              
==========================================
- Hits          532      527       -5     
- Misses        237      246       +9     
+ Partials       41       40       -1     
Impacted Files Coverage Δ
src/tinuous/__main__.py 48.87% <0.00%> (-0.38%) ⬇️
src/tinuous/util.py 84.53% <100.00%> (+0.32%) ⬆️
src/tinuous/appveyor.py 74.39% <0.00%> (-4.88%) ⬇️
src/tinuous/github.py 48.74% <0.00%> (-1.51%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd48e39...0bb368d. Read the comment docs.

@yarikoptic
Copy link
Member

Thank you @jwodder!

@yarikoptic yarikoptic merged commit 8902437 into master Jun 10, 2021
@yarikoptic yarikoptic deleted the gh-82 branch June 10, 2021 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Produced by tinuous {__version__}` at the bottom of datalad commit messages
3 participants