-
Notifications
You must be signed in to change notification settings - Fork 1
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: use manifest-beta
for BRAT pre-releases
#1
base: main
Are you sure you want to change the base?
Conversation
- use the `manifest-beta.json` for pre-releases - leave `manifest.json` untouched for pre-releases
…ase of pre-release
I submitted TfTHacker/obsidian42-brat#93 to |
manifest-beta
for BRAT pre-releasesmanifest-beta
for BRAT pre-releases
This looks great, thanks for the contribution! I'd be happy to merge this in but please add a I'm not a big user of BRAT, so let me know what'd improve your experience the most. Thanks again and happy new years! 🎉 |
Heads up I merged in some changes to your branch so that it's passing my checks! |
TODO(TfTHacker/obsidian42-brat#93): Make sure to remove code if BRAT integrates changes to use GitHub pre-releases
@brianrodri I have continued playing around with the plugin. It's really great! I have written up a small Gist with a sample I also wrote up a small README.md on Using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks @johannrichard! I've gone ahead and given the code changes a genuine review, PTAL!
I'll also play around with the .yml snippet on a private repository to quickly confirm, then I'll open a PR myself with the changes. Would you mind reviewing those changes when I get around to it?
Finally, thanks again!!
if (prerelease === false) { | ||
return false; | ||
} | ||
return ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a little complicated, could you please split these into a chain of if-else with brief comments describing how a user might "land" in each branch?
Also, please add some unit tests to demonstrate the same ideas (and so codecov stays at 100% 😊)
Add some more detail on pre-release branches.
The plugin must be specified as `semantic-release-obsidian-plugin` in the `plugins` list. Otherwise, `semantic-release` will fail.
PS: I saw there's a longer thread in the Obsidian Forum on GitHub actions. None of them mention your Please let me know what you think of a) and b). |
Haha b) sounds good to me! They'll need a link to this repo to file feature requests anyway 😎 |
Thanks for a great plugin! I implemented a small change that will create a
manifest-beta.json
in case a pre-release is detected (e.g. when committing to abeta
ornext
branch). This will then create amanifest-beta.json
for that pre-release and commit it to the branch, but will leave themanifest.json
untouched.Because of the way BRAT (currently) works, you either have to set your pre-release branch as the default branch and update it's
manifest.json
manually whenever a new version is published, or copy themanifest-beta.json
from your pre-release branch to the default branch.I'm not sure which one is the best way, or if there are better ways to solve this (including updating BRAT such that a branch could be specified instead of
manifest-beta.json
or a fixed version number).