Allow for defining arbitrary project version and name in configuratio… by fizyk · Pull Request #166 · twisted/towncrier

fizyk · 2019-11-12T15:23:25Z

…n - close #165

This allows for the towncrier to be seamlessly used in non-python projects

codecov-io · 2019-11-12T15:25:30Z

Codecov Report

Merging #166 (215a80a) into master (0df024e) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   89.49%   89.59%   +0.09%     
==========================================
  Files          11       11              
  Lines         438      442       +4     
  Branches       87       89       +2     
==========================================
+ Hits          392      396       +4     
  Misses         25       25              
  Partials       21       21

Impacted Files	Coverage Δ
src/towncrier/_settings.py	`95.94% <ø> (ø)`
src/towncrier/build.py	`89.87% <100.00%> (+0.54%)`	⬆️

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 a651d17...7adc1d6. Read the comment docs.

tjanez · 2019-12-20T22:40:13Z

We've successfully cherry-picked the changes in this PR to Oasis Labs' towncrier fork and used it with the Oasis Core repo, which is a Go & Rust project.

altendky

Also needs a catch up. Anyways, thanks for the work. Sorry I went and duplicated it, though that made the review easy since they were so close. :] I can't merge anything right now but I'll try to get that fixed so we can get this integrated once we work through the minor tidbits. My apologies it has taken so long to even get any feedback.

altendky · 2020-11-25T03:47:01Z

src/towncrier/_settings.py

+        "project_version": config.get("project_version"),
+        "project_name": config.get("project_name"),


Do the names in the configuration file need the project_ prefix? Or would name and version suffice.

I think it's a matter of preference, and short version is as good as longer. One does not use library_version or library_name variables after all...

Although comming back, I used the project_ prefixes since I was extending the use of already existing variables in code

Yeah, I saw the code used longer names and that makes sense to me. I couldn't make up my mind so I thought about setuptools and checked Poetry and both of them use just name and version. I don't think it ends up being ambiguous. I don't feel strongly enough to push hard on this, but if you are comfortable with changing to the shorter, I think that's a touch more straightforward. Take your pick and we'll move on.

ok, changed to shorter

.gitignore

src/towncrier/newsfragments/165.feature.rst

…n - close twisted#165 This allows for the towncrier to be seamlessly used in non-python projects

Co-authored-by: Kyle Altendorf <sda@fstab.net>

altendky

I started looking through the readme and found a few other references to 'version' that seemed like they could use some updating to account for this new feature.

So I just realized that this also introduces the possibility of specifying the version and the name via both the command line options and the configuration file. I think that other options, at least on the build command, can't do that. I vaguely feel like perhaps the CLI options should go away, though there would be a deprecation period for that. Let's just ignore that for now.
It can be handled in a separate PR if needed. But, it would probably be good to add tests that confirm the priority of the command line over the configuration file.

With all this time since you submitted this, I'm glad you are available to get it worked through and finished up quickly. :] I'm sorry I keep finding tidbits. I'm still on the newer side of review and appreciate your patience.

README.rst

altendky · 2020-11-25T23:17:46Z

src/towncrier/_settings.py

+        "project_version": config.get("project_version"),
+        "project_name": config.get("project_name"),


Yeah, I saw the code used longer names and that makes sense to me. I couldn't make up my mind so I thought about setuptools and checked Poetry and both of them use just name and version. I don't think it ends up being ambiguous. I don't feel strongly enough to push hard on this, but if you are comfortable with changing to the shorter, I think that's a touch more straightforward. Take your pick and we'll move on.

Co-authored-by: Kyle Altendorf <sda@fstab.net>

README.rst

fizyk · 2020-11-26T20:58:04Z

Also, I'd gladly squash commits and rework commit message if that'd be something you want.

altendky · 2020-11-27T00:59:42Z

My personal preference is to never change public history, so no need to go squashing etc for my sake.

altendky

There are still a few references to 'version' in the readme that will mislead people into thinking there isn't a new option for specifying it. If you would rather, I could go through that. Just let me know.

README.rst

altendky

Also see ClearcodeHQ#2 since GitHub didn't let me add a suggestion for a line not already in the diff context.

I think maybe that's it.

src/towncrier/newsfragments/165.feature.rst

Co-authored-by: Kyle Altendorf <sda@fstab.net>

altendky · 2020-12-02T18:47:43Z

@fizyk looks like you lost most of the changes in the readme. 215a80a

Co-authored-by: Kyle Altendorf <sda@fstab.net>

fizyk · 2020-12-02T22:10:40Z

@fizyk looks like you lost most of the changes in the readme. 215a80a

🤦🏻‍♂️ fixed it

altendky · 2020-12-07T03:43:36Z

@fizyk, sorry for the delay here. Looks good to me. I've got the CI fixed up properly now so I'm going to go ahead and update the branch here from master. Hopefully that'll just work and I'll merge this but I wanted to give you a heads up so you don't go trying to push new work without pulling and get a surprise.

fizyk · 2020-12-07T09:31:48Z

@altendky thank you!

tjanez mentioned this pull request Nov 27, 2019

Versioned Build/Release Process oasisprotocol/oasis-core#2419

Closed

tjanez mentioned this pull request Dec 9, 2019

Automatically generate the changelog oasisprotocol/oasis-core#759

Closed

altendky mentioned this pull request Nov 25, 2020

Allow specifying project name in config file #271

Closed

altendky requested changes Nov 25, 2020

View reviewed changes

Grzegorz Śliwiński added 2 commits November 25, 2020 22:49

Allow for defining arbitrary project version and name in configuratio…

e5b01bd

…n - close twisted#165 This allows for the towncrier to be seamlessly used in non-python projects

add tests

9957da2

fizyk force-pushed the allow_for_non_python_projects branch from 2a1e0b0 to 9957da2 Compare November 25, 2020 21:49

Update src/towncrier/newsfragments/165.feature.rst

97e08ec

Co-authored-by: Kyle Altendorf <sda@fstab.net>

altendky reviewed Nov 25, 2020

View reviewed changes

Grzegorz Śliwiński and others added 3 commits November 26, 2020 21:37

Update README.rst

4142edf

Co-authored-by: Kyle Altendorf <sda@fstab.net>

Use shorter config option names for version and name

8ac988c

Updated readme

fa8a66f

fizyk commented Nov 26, 2020

View reviewed changes

README.rst Show resolved Hide resolved

fizyk requested a review from altendky November 27, 2020 12:28

altendky reviewed Nov 27, 2020

View reviewed changes

README.rst Show resolved Hide resolved

altendky requested changes Dec 2, 2020

View reviewed changes

src/towncrier/newsfragments/165.feature.rst Outdated Show resolved Hide resolved

Grzegorz Śliwiński and others added 2 commits December 2, 2020 10:22

Update src/towncrier/newsfragments/165.feature.rst

75f54c8

Co-authored-by: Kyle Altendorf <sda@fstab.net>

Merge branch 'master' into allow_for_non_python_projects

a2bb865

fizyk mentioned this pull request Dec 2, 2020

Tweak ClearcodeHQ/towncrier#2

Closed

fizyk requested a review from altendky December 2, 2020 09:33

Update README.rst

833c8cf

Co-authored-by: Kyle Altendorf <sda@fstab.net>

fizyk force-pushed the allow_for_non_python_projects branch from 215a80a to 833c8cf Compare December 2, 2020 22:09

Merge branch 'master' into allow_for_non_python_projects

7adc1d6

Merge branch 'master' into allow_for_non_python_projects

af4056b

altendky approved these changes Dec 7, 2020

View reviewed changes

altendky merged commit 441ef3f into twisted:master Dec 7, 2020

fizyk deleted the allow_for_non_python_projects branch December 7, 2020 09:31

altendky mentioned this pull request Jul 16, 2021

Towncrier project name in config altendky/qtrio#89

Merged

2 tasks

		"project_version": config.get("project_version"),
		"project_name": config.get("project_name"),

Conversation

fizyk commented Nov 12, 2019

Uh oh!

codecov-io commented Nov 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tjanez commented Dec 20, 2019

Uh oh!

altendky left a comment

Choose a reason for hiding this comment

Uh oh!

altendky Nov 25, 2020

Choose a reason for hiding this comment

Uh oh!

fizyk Nov 25, 2020

Choose a reason for hiding this comment

Uh oh!

fizyk Nov 25, 2020

Choose a reason for hiding this comment

Uh oh!

altendky Nov 25, 2020

Choose a reason for hiding this comment

Uh oh!

fizyk Nov 26, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

altendky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

altendky Nov 25, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fizyk commented Nov 26, 2020

Uh oh!

altendky commented Nov 27, 2020

Uh oh!

altendky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

altendky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

altendky commented Dec 2, 2020

Uh oh!

fizyk commented Dec 2, 2020

Uh oh!

altendky commented Dec 7, 2020

Uh oh!

fizyk commented Dec 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Nov 12, 2019 •

edited

Loading