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!: changes default revision from master to main #951

Merged
merged 1 commit into from
Nov 25, 2022

Conversation

subzero10
Copy link
Member

Status

READY

Description

Closes: #948

@subzero10 subzero10 added the webpack @honeybadger-io/webpack label Nov 18, 2022
@subzero10 subzero10 self-assigned this Nov 18, 2022
@shalvah
Copy link
Contributor

shalvah commented Nov 18, 2022

Hmm, I don't know about this. There are still many projects that use master. Considering that main is only around 2 years old as a default, it's definitely in the minority. 🤔

@KonnorRogers
Copy link
Collaborator

KonnorRogers commented Nov 20, 2022

To me this feels like a BREAKING CHANGE rather than a feature, otherwise 👍 from me.

@subzero10
Copy link
Member Author

Hmm, I don't know about this. There are still many projects that use master. Considering that main is only around 2 years old as a default, it's definitely in the minority. 🤔

With the same argument, can't we say that since the default has been main for around 2 years, it is time to make it default as well?

@subzero10
Copy link
Member Author

To me this feels like a BREAKING CHANGE rather than a feature, otherwise 👍 from me.

I had the same thought, but then again I don't know how big the impact would be with this change.
What do you think @BethanyBerkowitz and @shalvah ?

@shalvah
Copy link
Contributor

shalvah commented Nov 20, 2022

With the same argument, can't we say that since the default has been main for around 2 years, it is time to make it default as well?

Nah. master has been the default for decades. Heck, this repository still uses it. Not saying we can't change it, but it's not a flippant decision.

To me this feels like a BREAKING CHANGE rather than a feature

Ah, I didn't think about it, but it's true. if someone has been using this plugin with the default config, wouldn't that mean their sourcemaps suddenly get uploaded under a different revision? I don't know much about what happens after that, but might that not break stuff? (Personally, even if it's a new major, I don't think it's worth it.)

@subzero10
Copy link
Member Author

Nah. master has been the default for decades. Heck, this repository still uses it. Not saying we can't change it, but it's not a flippant decision.

True! Just to be consistent, I think we should change to main branch with this repo too (we did it for most of the other honeybadger repos).

I don't know much about what happens after that, but might that not break stuff? (Personally, even if it's a new major, I don't think it's worth it.)

I don't know. @stympy , do you have any thoughts on this?
We can always go for the safer path and just release a major version 🤔 .

@stympy
Copy link
Member

stympy commented Nov 21, 2022

If you take a look at our docs, we specifically warn against leaving the default of "master" in place, so perhaps a change to main wouldn't be a huge deal. It would change mappings for people who just go with the default, but if they care about mappings, I'm thinking they won't be sticking with the default. I bet @joshuap has some valuable input on this. 😉

@joshuap
Copy link
Member

joshuap commented Nov 22, 2022

If you take a look at our docs, we specifically warn against leaving the default of "master" in place, so perhaps a change to main wouldn't be a huge deal. It would change mappings for people who just go with the default, but if they care about mappings, I'm thinking they won't be sticking with the default. I bet @joshuap has some valuable input on this. 😉

This is correct. People really shouldn't be using this default setting. It makes source maps much flakier. That said, I'm sure there are people who are using it in practice. For that reason, maybe it should be a breaking change. :)

I guess one drawback of the monorepo is that if you release a breaking change in a minor package, you have to release a new major version of all packages—is that right @subzero10?

@stympy
Copy link
Member

stympy commented Nov 22, 2022

I'm all for switching to main, since GitHub is using it as the default, thus all new projects will be defaulting to it. Based on the conversation we've had here, it sounds like that change should be part of a major release, which works for me. It would be nice, though, if it weren't the only reason for a major release... it would be nice to have something else major to go into that release. ;)

@joshuap
Copy link
Member

joshuap commented Nov 22, 2022

I'm all for switching to main, since GitHub is using it as the default, thus all new projects will be defaulting to it. Based on the conversation we've had here, it sounds like that change should be part of a major release, which works for me. It would be nice, though, if it weren't the only reason for a major release... it would be nice to have something else major to go into that release. ;)

We do have #918 😁

Edit: We could add both of these PRs to a milestone and then see if there are any other major changes we want to include before releasing the next major version.

@stympy
Copy link
Member

stympy commented Nov 22, 2022

We do have #918 😁

Perfect :)

@subzero10
Copy link
Member Author

subzero10 commented Nov 22, 2022

I guess one drawback of the monorepo is that if you release a breaking change in a minor package, you have to release a new major version of all packages—is that right @subzero10?

It's a drawback of our current monorepo setup. We knowingly took this decision to help with the monorepo transition. I can revisit versioning and see what it means to support independent versioning. FYI, sentry does not do independent versioning. I wonder if they have a reason that we didn't experience yet 🤔

Edit: We could add both of these PRs to a milestone and then see if there are any other major changes we want to include before releasing the next major version.

Note that we have the weekly automatic release on the js monorepo. So if we want to control the next release, we have to remember to disable the release workflow (it's just a toggle on the UI).

@joshuap
Copy link
Member

joshuap commented Nov 22, 2022

Fwiw, I'm not saying we should change the current setup to do independent versioning, as long as we can work around it and have a way to plan larger releases so that we're constantly bumping the major version. For instance, could we stage the next major release on a branch, and then merge the branch when we're ready to release everything?

@subzero10 subzero10 changed the title feat: changes default revision from master to main feat!: changes default revision from master to main Nov 25, 2022
@subzero10 subzero10 changed the base branch from master to releases/v5 November 25, 2022 12:44
@subzero10 subzero10 added this to the v5 milestone Nov 25, 2022
@subzero10
Copy link
Member Author

Fwiw, I'm not saying we should change the current setup to do independent versioning, as long as we can work around it and have a way to plan larger releases so that we're constantly bumping the major version. For instance, could we stage the next major release on a branch, and then merge the branch when we're ready to release everything?

Done! I marked the PR as breaking change (see ! on PR title), switched base to releases/v5 and created a v5 milestone.

@subzero10 subzero10 merged commit 89d6184 into releases/v5 Nov 25, 2022
@subzero10
Copy link
Member Author

See #959.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
webpack @honeybadger-io/webpack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[webpack] Change default revision and examples from "master" to "main"
5 participants