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

Define a user-visible change #9946

Merged
merged 2 commits into from
May 1, 2024
Merged

Conversation

philderbeast
Copy link
Collaborator

In support of #9944, define what a user-visible change is in CONTRIBUTING.md.

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

Copy link
Collaborator

@ffaf1 ffaf1 left a comment

Choose a reason for hiding this comment

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

Do you think you can make this a subsection? So we can directly link to this?

@philderbeast
Copy link
Collaborator Author

@ffaf1
Copy link
Collaborator

ffaf1 commented Apr 28, 2024

Firefox 115 mangles it (goes a few lines too far)

image

@philderbeast
Copy link
Collaborator Author

philderbeast commented Apr 28, 2024

Dang, Firefox 125.0.2 on ubuntu does fine with it and the same as chromium, bringing the whole sentence into view at the top of the frame with the anchor underlined.

@geekosaur
Copy link
Collaborator

Same with Chrome 124.0.6367.91 (works fine).

@philderbeast philderbeast force-pushed the docs/user-visible-is branch 4 times, most recently from 1e40d2e to b4e6a1b Compare April 28, 2024 19:12
@ffaf1
Copy link
Collaborator

ffaf1 commented Apr 28, 2024

If it is the first paragraph in that section, a link to #changelog will be fine!

@philderbeast philderbeast force-pushed the docs/user-visible-is branch from b4e6a1b to d65a4a5 Compare April 28, 2024 19:38
@philderbeast
Copy link
Collaborator Author

philderbeast commented Apr 28, 2024

@ffaf1
Copy link
Collaborator

ffaf1 commented Apr 28, 2024

Very good!

@ffaf1 ffaf1 mentioned this pull request Apr 28, 2024
2 tasks
@philderbeast philderbeast force-pushed the docs/user-visible-is branch from d65a4a5 to 8b9e742 Compare April 28, 2024 20:11
@philderbeast philderbeast requested a review from ffaf1 April 28, 2024 20:11
@philderbeast
Copy link
Collaborator Author

Label merge+no rebase is necessary when the pull request is from an organisation.

@Mikolaj
Copy link
Member

Mikolaj commented Apr 29, 2024

It's just been pointed out to me in #9952 (comment) that dropping support of a GHC version (by removing it from CI, which should be accompanied by revising a base bound) is a crucial change for some users. Could we incorporate that one into the needs-changelog guideline (maybe directly into the user-visible definition?)?

@ffaf1
Copy link
Collaborator

ffaf1 commented Apr 29, 2024

Could we incorporate that one into the needs-changelog guideline (maybe directly into the user-visible definition?)?

I put this in the “Making a release document”, do you feel it is better to have it there (one-off activity every nine months) or here (instruction to each PR). I am OK in any case.

- Add anchor for user-visible change
@philderbeast philderbeast force-pushed the docs/user-visible-is branch 2 times, most recently from 9865bee to 5b71c0f Compare April 29, 2024 16:38
@philderbeast
Copy link
Collaborator Author

philderbeast commented Apr 29, 2024

It's just been pointed out to me in #9952 (comment) that dropping support of a GHC version (by removing it from CI, which should be accompanied by revising a base bound) is a crucial change for some users. Could we incorporate that one into the needs-changelog guideline (maybe directly into the user-visible definition?)?

@Mikolaj I've added something about this;

Raising the lower bound on base is most definitely a user-visible change because it
excludes versions of GHC from being able to build these packages.

@philderbeast philderbeast force-pushed the docs/user-visible-is branch from 5b71c0f to c0ba553 Compare April 29, 2024 16:40
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

LGTM

@Mikolaj
Copy link
Member

Mikolaj commented Apr 29, 2024

@philderbeast: thank you

@ffaf1: I'm lazy so I'd rather PR authors do all the work in the chagelog files. However, the recent experience shows we need to monitor that PRs get proper changelogs, so IMHO this bit also makes sense in the major release checklist (especially that it's rather cheap to compare CI scripts and base bounds with what was there in the previous major release, as opposed to going through all PRs and verifying they don't break API silently).

@ffaf1
Copy link
Collaborator

ffaf1 commented Apr 29, 2024

so be it, then let’s update it here and I will slightly reword “Making a release”.

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label May 1, 2024
@mergify mergify bot merged commit 8bde3a6 into haskell:master May 1, 2024
11 checks passed
@Mikolaj
Copy link
Member

Mikolaj commented May 1, 2024

I don't think we need to backport that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge+no rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants