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(updater): add YAML support (#33, #748) #137

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

danielgrad
Copy link

Adding support for YAML files. Based on conventional-changelog#748. Geared towards Dart/Flutter pubspec.yaml files, but will work with any YAML file using root level version key.

@TimothyJones
Copy link
Member

Thanks so much for this!

@TimothyJones
Copy link
Member

npm run format:fix will fix the formatting issues - I tried to push a fix commit to your branch, but it seems I don't have permissions.

Anyway, this looks great - thanks for the feature, I'll merge it once it passes the format check

@danielgrad
Copy link
Author

Fixed formatting issues.

@TimothyJones
Copy link
Member

I suspect the failure might be due to line ending issues. I’m travelling this week so can’t take a detailed look at the moment, sorry

@danielgrad
Copy link
Author

updated to preserve line endings

@ixuz
Copy link

ixuz commented Apr 17, 2024

Could this yaml version updater perhaps also support updating versions that are located deeper in the yaml document?

If that would be the case, then this PR could supersede the PR that I opened earlier. Which needs to bump the version located under property: info.version.

I don't want to scope creep your work, so maybe it's better to work this detail out under a separate issue later (after this is PR has been merged).

What do you think?

@danielgrad
Copy link
Author

@ixuz I saw your PR already and I wanted to try to integrate a way to do that, but I did not see an easy way to add additional options to the command (to provide a specific path to update). Maybe there is and I missed it.

@TimothyJones
Copy link
Member

Ah, apologies - I missed the notification about that PR for some reason. I've been travelling for the past few weeks, back on Friday. I'll take a quick look now.

The options system is a bit annoying in that it isn't clearly separated from the config - so not all options and ways of setting them are equal. This also makes it harder to add new ways of config, as there isn't a super consistent pattern.

There have been a couple of efforts to address this (the last one stalled I believe, possibly because introducing typescript at the same time made it a larger effort).

so maybe it's better to work this detail out under a separate issue later (after this is PR has been merged).

I think this makes sense - my gut feel is that it would be nice to have a general yaml support, and then perhaps a few specific ones (eg openapi) that could be just specific configurations of the yaml plugin, maybe? I don't know.

@TimothyJones
Copy link
Member

Thanks both for your excellent work, by the way! It's a really nice maintainer experience to receive PRs that include documentation and tests 🙌 🙌

@TimothyJones TimothyJones merged commit b9dccc2 into absolute-version:master Apr 18, 2024
8 checks passed
@TimothyJones
Copy link
Member

Released as 12.3.0. Thanks again!

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.

None yet

3 participants