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: allow configuring simple strategy version file #1168

Merged
merged 4 commits into from
Jan 12, 2022

Conversation

chingor13
Copy link
Contributor

Fixes #921

@chingor13 chingor13 requested a review from a team December 23, 2021 00:28
@chingor13 chingor13 requested a review from a team as a code owner December 23, 2021 00:28
@goatwu1993
Copy link
Contributor

goatwu1993 commented Dec 24, 2021

Please ignore. Off topic.

Any chance simple strategy to take a list of string and use ["version.txt"] as default values?

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.

Thinking outloud, could the versionFile be a JSON file, and if so we take a key to update?

@axieum
Copy link
Contributor

axieum commented Dec 29, 2021

Perhaps the generic updaters (#1157) could take a replacement method, e.g.

{
  "extra-files": [
    {
      "path": "version.json",
      "method": "regex",
      "version": ["\"version\": \"(.+?)\""], // arg for full version
      "minor": ["\"minor\": \"(.+?)\""] // arg for minor, can add more for major, patch, build, etc.
    },
    {
      "path": "some.txt",
      "method": "annotation" // optional/default
    },
    {
      "path": "other.json",
      "method": "json",
      "major": "version.items[2].major" // arg for major
  ]
}

I'm not sure how this would be set on the CLI though if wanting to allow configuration through that as well?

@chingor13
Copy link
Contributor Author

Perhaps the generic updaters (#1157) could take a replacement method, e.g.

{
  "extra-files": [
    {
      "path": "version.json",
      "method": "regex",
      "version": ["\"version\": \"(.+?)\""], // arg for full version
      "minor": ["\"minor\": \"(.+?)\""] // arg for minor, can add more for major, patch, build, etc.
    },
    {
      "path": "some.txt",
      "method": "annotation" // optional/default
    },
    {
      "path": "other.json",
      "method": "json",
      "major": "version.items[2].major" // arg for major
  ]
}

I'm not sure how this would be set on the CLI though if wanting to allow configuration through that as well?

Thinking outloud, could the versionFile be a JSON file, and if so we take a key to update?

I think the issue both of you are describing is more related to #1180 than this fix. The versionFile here is a simple text file that only contains the version number.

@@ -36,7 +45,7 @@ export class Simple extends BaseStrategy {
});

updates.push({
path: this.addPath('version.txt'),
path: this.addPath(this.versionFile),
Copy link
Contributor

Choose a reason for hiding this comment

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

nice 🥳

Is it worth adding a test that we default to version.txt?

@chingor13 chingor13 added the automerge Merge the pull request once unit tests and other checks pass. label Jan 12, 2022
@chingor13 chingor13 merged commit 08a0cf2 into main Jan 12, 2022
@chingor13 chingor13 deleted the customize-simple-version-file branch January 12, 2022 20:55
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simple release type: support version-file config option
4 participants