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: introduce generic updater #1157

Merged
merged 10 commits into from
Dec 29, 2021
Merged

feat: introduce generic updater #1157

merged 10 commits into from
Dec 29, 2021

Conversation

chingor13
Copy link
Contributor

@chingor13 chingor13 commented Dec 21, 2021

This introduces a new Generic updater class that scans the file line by line for annotations.

You can annotate a line (inline) via:

  • x-release-please-version
  • x-release-please-major
  • x-release-please-minor
  • x-release-please-patch

For these annotations, we will try to replace the value on that line only.

You can annotate a block by starting with a line containing:

  • x-release-please-start-version
  • x-release-please-start-major
  • x-release-please-start-minor
  • x-release-please-start-patch

and close the block with a line containing x-release-please-end. Within the block, we will attempt to replace version values.

Additionally, all basic strategies now support the extra-files option which will apply this generic updater class.

Fixes #435
Fixes #305
Fixes #1139
Fixes #1174

@@ -40,3 +40,27 @@ export class CompositeUpdater implements Updater {
return content || '';
}
}

export function mergeUpdates(updates: Update[]): Update[] {
const updatesByPath: Record<string, Update[]> = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to have Record<Update['path'], Update[]>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would probably work in case update's path's type changes, but I think for readability, string might be easier to read.

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.

I like the idea of annotations. In practice, has is the list of files provided to update?

__snapshots__/generic.js Show resolved Hide resolved
@chingor13
Copy link
Contributor Author

I like the idea of annotations. In practice, has is the list of files provided to update?

For the generic updater, this will be provided adhoc via the extraFiles factory option or extra-files CLI/manifest option.

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.

I like this idea, but I think it would be good to ad a docs/customizing-releases.md or something along those lines, where you speak to how someone can configure and use this functionality.

@chingor13
Copy link
Contributor Author

I like this idea, but I think it would be good to ad a docs/customizing-releases.md or something along those lines, where you speak to how someone can configure and use this functionality.

Opened: #1174 to follow up with

@chingor13 chingor13 requested a review from bcoe December 29, 2021 21:28
@chingor13
Copy link
Contributor Author

@bcoe Added documentation in docs/customizing.md

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.

Seems like a great addition to functionality, and we can evolve the feature over time if we find supporting JSON files is a requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants