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

feat: add ability to specify extra files for releasers to consider #850

Merged
merged 2 commits into from Apr 9, 2021

Conversation

chingor13
Copy link
Contributor

Currently, only Java utilizes this option, but this could be used by
any release strategy.

Towards #305
Towards #597

Currently, only Java utilizies this option, but this could be used by
any release strategy.
@chingor13 chingor13 requested a review from a team April 9, 2021 17:19
@chingor13 chingor13 requested a review from a team as a code owner April 9, 2021 17:19
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 9, 2021
@chingor13
Copy link
Contributor Author

chingor13 commented Apr 9, 2021

cc @igorbernstein2 This would be a more generalized solution for being able to make replacements in one-off files (without hard-coding a list)

@codecov
Copy link

codecov bot commented Apr 9, 2021

Codecov Report

Merging #850 (41fa44a) into master (24f81b1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   93.77%   93.78%   +0.01%     
==========================================
  Files          64       64              
  Lines        9203     9218      +15     
  Branches      936      992      +56     
==========================================
+ Hits         8630     8645      +15     
  Misses        570      570              
  Partials        3        3              
Impacted Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)
src/release-pr.ts 94.51% <100.00%> (+0.02%) ⬆️
src/releasers/java-yoshi.ts 90.76% <100.00%> (+0.29%) ⬆️

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 24f81b1...41fa44a. Read the comment docs.

@@ -299,6 +299,18 @@ export class JavaYoshi extends ReleasePR {
})
);
});

this.extraFiles.forEach(path => {
Copy link
Contributor

Choose a reason for hiding this comment

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

my slight concern here is behavior that's one-language specific, perhaps add a TODO to flesh out this behavior. I'm imagining you'd eventually pass a file plus the updater to apply?

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 think that will be too much config. The general use case for extra files would be one-offs that update the version strings in a file. Each language will want a generic way to do this - we don't want a bunch of one-off updaters for this.

Copy link
Contributor

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

👍 with the note that I'd like to minimize language specific features if we can make them more generic.

@chingor13 chingor13 merged commit f7079fd into googleapis:master Apr 9, 2021
@chingor13 chingor13 deleted the extra-files branch April 9, 2021 20:15
gcf-merge-on-green bot pushed a commit that referenced this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants