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

ci: add next-major branch to the list of branches to release #442

Merged
merged 3 commits into from
Feb 7, 2022

Conversation

smoya
Copy link
Member

@smoya smoya commented Jan 12, 2022

Description

Relates to #401.
This change will allow next-major branch to be released on each merge, so users can use and test the new work during the development phase.

Related issue(s)
#401

cc @derberg @jonaslagoni

package.json Outdated
Comment on lines 94 to 96
{
"name": "intent-api",
"prerelease": true
Copy link
Member

Choose a reason for hiding this comment

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

Can CI handle double prerelease @derberg? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

yes, it is ready as it was required for React component and the 2.0 prerelease phase with next branch.

We only need to agree on bumping strategy. In React for example every single release candidate goes to html-template, so on master we had to make sure that whenever you release to master, main release is not bumped in html-template -> https://github.com/asyncapi/asyncapi-react/blob/master/.github/workflows/bump.yml#L32

Copy link
Member

Choose a reason for hiding this comment

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

@jonaslagoni but I guess we do not want to bump it automatically anywhere. I don't see a point at this moment

Copy link
Member

Choose a reason for hiding this comment

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

What about the generator? Had in mind we integrated it for templates to use the new API ASAP.

Copy link
Member

Choose a reason for hiding this comment

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

all templates follow guidelines to set that they are always compatible with generator prior 2.0
so yeah, pushing new Parser would be breaking change, unless you are thinking about release candidates in generator too 🤔

Copy link
Member

Choose a reason for hiding this comment

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

well, I love the idea!
but 😄 I don't think my bumper supports custom branches 😆

Copy link
Member

Choose a reason for hiding this comment

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

@smoya ping as not sure if you saw it. I think this one is not just nice to have, but tbh a must-have, otherwise testing new functions in templates will be complicated

Copy link
Member

@derberg derberg Feb 5, 2022

Choose a reason for hiding this comment

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

@smoya @jonaslagoni good news, I just released v4 of the bumper action

https://github.com/derberg/npm-dependency-manager-for-your-github-org

Now it supports new prop for config - base_branch

So once we merge this PR, and create branch next-major, please open another PR against this new release branch and make sure that the bump.yml workflow:

  • its version in next-major branch will react to commits to next-major and not master
  • add base_branch: next-major. This will force bumper to bump Parser in dependants that also have next-major branch, and PR with bump will be opened against next-major, which means that after merge the release candidate will be published for a given project, if configured

React component does it already, but base_branch is not used -> https://github.com/asyncapi/asyncapi-react/blob/next/.github/workflows/bump.yaml

makes sense?

Copy link
Member Author

@smoya smoya Feb 7, 2022

Choose a reason for hiding this comment

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

That's awesome! Yeah makes totally sense. Thanks for the work you did in the bumper action!
Let's do that thing you suggest then @derberg !

Copy link
Member Author

Choose a reason for hiding this comment

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

It's done here: #468 @derberg @jonaslagoni

package.json Outdated
@@ -90,6 +90,10 @@
{
"name": "2021-09-release",
"prerelease": true
},
{
"name": "intent-api",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"name": "intent-api",
"name": "next",

please rename it to next. This is standard recommended by semantic-release and npm. And release pipeline supports it.

also from user perspective, at the end it is about next major release. We will not release only intent API in 2.0 of the Parser, there are some other stuff I bet we want to push in

Copy link
Member

Choose a reason for hiding this comment

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

TS

Copy link
Member

Choose a reason for hiding this comment

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

anyway 🤷🏼

Copy link
Member

@jonaslagoni jonaslagoni Jan 12, 2022

Choose a reason for hiding this comment

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

Truly, TS is just far better at dictating types than JSDoc 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

please rename it to next. This is standard recommended by semantic-release and npm. And release pipeline supports it.

also from user perspective, at the end it is about next major release. We will not release only intent API in 2.0 of the Parser, there are some other stuff I bet we want to push in

Then I think it's better to use next-major as also specified in semantic-release doc.

Copy link
Member

Choose a reason for hiding this comment

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

but why 😄 if recommended is next 😄 and then you need to update global workflow everywhere https://github.com/asyncapi/.github/blob/master/.github/workflows/if-nodejs-release.yml#L12. Why not just next and follow the best practices we got from working with react already?

Screenshot 2022-01-12 at 18 11 10

@jonaslagoni actually I did not understand your question properly, now saw below after changes from Sergio.

      {
        "name": "2021-09-release",
        "prerelease": true
      },
      {
        "name": "next-major",
        "prerelease": true
      }

Tbh I'm not sure, we need to check docs of semantic-release package to be 100% sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

next-major is standard as well in semantic release. It is intended to be used for that, major releases, which it obviously fits better to our case. We wont block other future releases (minor or patches) may happen before the major.

I didnt checked the ci script you linked until this right moment.

Its not about discarding what you recommend to do randomly 😅. I just read your comment, learned about semantic release next branch mechanism, and thats where I found that the next-major tag its standard as well as release.

Copy link
Member Author

@smoya smoya Jan 12, 2022

Choose a reason for hiding this comment

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

Btw, I created this PR asyncapi/.github#121 so we add all default semantic-release branches to the release workflow. cc @derberg

Copy link
Member

Choose a reason for hiding this comment

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

@smoya did you maybe have a chance to check docs, of semantic-release if it comes to supporting multiple prereleases? I bet it is possible, but just to be 100% sure

Copy link
Member Author

@smoya smoya Jan 13, 2022

Choose a reason for hiding this comment

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

@smoya did you maybe have a chance to check docs, of semantic-release if it comes to supporting multiple prereleases? I bet it is possible, but just to be 100% sure

It does not explicitly say it, however, I understand it happens as I don't see why it wouldn't.
The release workflow will run against just one branch, so the script will check if the branch needs to be pre-released or not based on the prerelease: true property in package.json.

But in this case, why would we need a prerelease for the next-major branch?

@smoya smoya requested a review from derberg January 12, 2022 15:54
@smoya smoya changed the title ci: add intent-api branch to the list of branches to release ci: add next-major branch to the list of branches to release Jan 12, 2022
@smoya smoya requested a review from jonaslagoni February 7, 2022 12:50
package.json Outdated Show resolved Hide resolved
@smoya smoya requested a review from jonaslagoni February 7, 2022 13:44
@smoya
Copy link
Member Author

smoya commented Feb 7, 2022

/rtm

@smoya
Copy link
Member Author

smoya commented Feb 7, 2022

@asyncapi-bot why don't you merge my PR? :(

EDIT:

2022-02-07T19:29:24.895Z INFO  Current PR status: mergeable_state: behind
2022-02-07T19:29:24.895Z INFO  PR not ready to be merged after 20 tries

You bold @asyncapi-bot ❤️

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@smoya
Copy link
Member Author

smoya commented Feb 7, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit 8369b10 into asyncapi:master Feb 7, 2022
@smoya smoya deleted the intent-api branch February 7, 2022 20:08
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.15.0-2022-04-release.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants