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

refactor!: follow go semver #3793

Merged
merged 12 commits into from
Dec 5, 2023
Merged

refactor!: follow go semver #3793

merged 12 commits into from
Dec 5, 2023

Conversation

julienrbrt
Copy link
Member

@julienrbrt julienrbrt commented Dec 1, 2023

Prepare Ignite v28.

Did the following:

find . -type f \
    -name '*.go' \
    -exec sed -i -e 's,github.com/ignite/cli,github.com/ignite/cli/v28,g' {} \;

and edited the go.mod.

I now need to check the github actions and documentation to be sure it points to the proper way of installing and using Ignite (apis).

@julienrbrt julienrbrt marked this pull request as ready for review December 1, 2023 15:37
Copy link
Contributor

github-actions bot commented Dec 1, 2023

Visit the preview URL for this PR (updated for commit c4e9445):

https://igntservices-docs--pr3793-julien-v28-nyh6vomo.web.app

(expires Mon, 11 Dec 2023 20:34:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 95379efd94dd497aaa37c2d0354e6e2cafca5ec5

Co-authored-by: Jerónimo Albi <[email protected]>
Co-authored-by: Jerónimo Albi <[email protected]>
@julienrbrt julienrbrt enabled auto-merge (squash) December 4, 2023 13:54
@julienrbrt julienrbrt disabled auto-merge December 4, 2023 13:54
@julienrbrt julienrbrt enabled auto-merge (squash) December 4, 2023 13:55
@ilgooz ilgooz disabled auto-merge December 5, 2023 12:02
@ilgooz ilgooz merged commit 1b408b3 into main Dec 5, 2023
45 checks passed
@ilgooz ilgooz deleted the julien/v28 branch December 5, 2023 12:02
@tbruyelle
Copy link
Contributor

Sorry to come after the merge, but this change will cause a lot of merge conflicts, not to mention make the release process more tedious. What exactly is the benefit? Is there an intent behind this that I'm not aware of?

@julienrbrt
Copy link
Member Author

Why would it cause merge conflicts?
For the current PRs for sure, but I don't think it will afterwards.

We do need indeed such PR before each major release.

On the direction of moving to a major version, I'll defer to @ilgooz. However from I assure that it will make Ignite APIs easier to use.

@tbruyelle
Copy link
Contributor

Why would it cause merge conflicts?

Every time there is a new release and some PRs exist but are not mergeable at the time of the release. And I think this will continue for the next releases.

For the next release, the ignite package path will change from v28 to v29, so in addition to conflicts, we'll have to be careful. For example, an existing PR might inadvertently keep some v28 imports. I always try to avoid this kind of thing you might forget :)

I was also worried about the templates, but I realize that at least the ignite dependency is no longer part of a scaffolded chain. So far, so good.

@julienrbrt
Copy link
Member Author

julienrbrt commented Dec 12, 2023

Yeah, personally I am advocating the use of release branches, this may make the separation of concern and conflict limitation.
Like release/v28.x.x contains v28.x.x., then when we release, we push main to release/v28.x.x and already update main to v29.
Then we can backport anything for v28.x.x to release/v29.x.x and aren't limited to pushing breaking changes to main.

Right now, the flow is not working anymore when you want to push breaking changes to main (as releases are tagged there, and we follow go semver).

However, I know Ilker has other thoughts on that and would prefer v29.x.x feature branch (I guess it is a sort of development branch) instead with PRs targeting that branch instead, so we can keep tagging from main. The drawback is, once v29 is released we cannot maintain v28 anymore.

Whichever we pick, we do need to change the process anyway.

@tbruyelle
Copy link
Contributor

Yeah, personally I am advocating the use of release branches, this may make the separation of concern and conflict limitation.

💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:ci CI/CD workflow and automated jobs. component:cmd component:configs component:docs Documentation additions or improvements. component:packages component:templates type:internal type:services Service-related issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants