-
Notifications
You must be signed in to change notification settings - Fork 43
feat(updater): add maven pom.xml file support (#33) #109
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 maven pom.xml file support (#33) #109
Conversation
Nice work! Thanks so much for this! Looks like the build is failing due to an older node version - when I get a moment I'll bump the node versions in CI and release a breaking change - then you'll be able to rebase on that and it should work (assuming the tests work, but my experience is that PRs that come with tests and documentation generally pass). |
Ok, I've fixed the build - you'll be able to run the tests after a merge. However, I also noticed that many of the tests weren't actually running, because of dangling chai-as-promised calls. I fixed this by bringing in recommended lint settings for mocha and chai as promised (which also introduced a lot of merge conflict in the test, sorry). Now that the async test is running, it's sadly is failing due to:
|
…ersion into maven-file-support
Since the merge was kind of unpleasant, I've pushed a merge commit to your branch If you're able to fix up the test failure, I'll get this released. (Also, a note - due to lack of support in mock-fs, the tests won't work on node 20.5 or above) Edit: I just saw that I accidentally slipped in formatting changes from my usual editor settings - apologies. Feel free to have another go at the merge if you'd prefer a cleaner one. |
@TimothyJones Ill take a look later today after work and get everything fixed up. I was thinking about adding a |
I don't really maven, but as I understand it, snapshots are usually prereleases or something incremental like a nightly CI build (is that right?). Anyway,
I think there's probably an ergonomic argument for having Following the fork rationale, I'd be happy to accept a convenience option that made it easier to use semver and conventional commits in a maven world. Unless of course, I'm missing something, and it's really not a semver? |
I had a quick look at the maven docs - it looks like it's pretty liberal with versions, and If that's the case,
and both the build and pre-release identifiers are allowed to include It would change the semantics a bit, like you would parse eg |
A small tangent - if you want more meaningful incremental version numbers, that's the original use case for absolute-version. It works pretty well in tandem with commit-and-tag-version (and with a better understanding of how SNAPSHOT is meant to work in maven, I could probably improve that tool too). |
@paul-barton I've got a PR ready for updating the formatting in #114 . I'll wait for this to be ready first, though (or, alternatively, feel free to base it off that and re-raise. Up to you) |
#114 seems to have been merged. |
Do you happen to have any updates on this? |
This PR doesn't pass the tests at the moment, so I'm not able to merge it. I haven't heard from the author in several months, so I suspect it is abandoned. I'd be happy to accept another PR that provides the same feature, or if the author returns and fixes up this one, that would be fine too. |
@TimothyJones Okay, I understand! I have installed the @paul-barton's branch using I will make a fork of that branch and try to fix the code that isn't passing the tests, and then submit a new PR ;) |
Thanks to @TaylorHo , this has now been released as 12.2.0. Thanks very much to Taylor, and also to the original PR author @paul-barton . Much appreciated! |
@TaylorHo and @TimothyJones thank you for getting this merged and for all the help, I had some medical issues occur and wasn't able to get back to this PR. Feeling better now, but Thank you again! |
Welcome back! We're glad you're feeling better, @paul-barton! |
Great! Thanks all :) |
Adding Maven
pom.xml
File Support in #33