Skip to content

Conversation

JordanMartinez
Copy link
Contributor

This PR adds all the changelog entries across core, contrib, web, and node libraries from the v0.15.0 ecosystem update. Only relevant (i.e. useful to end-users, not necessarily maintainers) content is included.

The content here can and should diverge from the original changelog entry when this version can provide more context/instructions on how to migrate things well.

Unresolved questions/issues:

  • Does foreign-object need any of its Semigroup/Monoid instances added/changed to be consistent with Map's changes?
  • I noticed that purescript-numbers' changelog entry is missing the one about FFI migration. Resolving a merge conflict may have removed that file.
  • Affjax's notes will need to be updated since that library still isn't finished yet.
  • Dropping support for color schemes in the 'colors' repo wasn't included in its changelog entry because it was already done in a prior release. Should it be added here?

@thomashoneyman
Copy link
Member

Dropping support for color schemes in the 'colors' repo wasn't included in its changelog entry because it was already done in a prior release. Should it be added here?

I don’t think we should include this, both because it happened in a prior release and because it’s only just become a contributor library.

In general I think we should only include changes that we think are likely to affect a good number of users (ie. a small breaking change to purescript-uri wouldn’t need to be included), and we should prioritize core libraries over contrib libraries — officially, the core team only updates core libraries for the release. We just update the others to make it a smooth transition.

- Drop deprecated `group'` and `empty` (#219 by @JordanMartinez)

Other improvements:
- Fixed minor documention issue with `find` (#216 by @JamieBallingall)
Copy link
Member

Choose a reason for hiding this comment

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

I don’t think we should include small things like this, especially because it isn’t breaking. You can always look at libraries you use to look for stuff like this. To keep the migration guide reasonable in length I think we should prioritize changes that are breaking (you HAVE to change your code) or likely to affect many people (you probably WANT to change your code)

### purescript-control

Breaking changes:
- Make `<|>` right associative (#80 by @JordanMartinez)
Copy link
Member

@thomashoneyman thomashoneyman Mar 26, 2022

Choose a reason for hiding this comment

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

This, for example, does make sense to me to stay in the migration guide. But we aren’t explaining what to do about it at all — if we’re going to include a change in the guide then we should tell you what you might need to do. For example, this change could mention that it led to changes in associativity or precedence for quickcheck and string-parsers and parsing

If you look at last year’s migration guide you can see that the changes aren’t just a list — they include why the change was made and what you should do about it

@thomashoneyman
Copy link
Member

In keeping with my review comments above, I think this is way too much content for the guide and we need to trim it back.

It may be that we should create a separate document that aggregates changelogs across all libraries (which is how this content reads). That seems valuable to me.

But I don’t think it’s the right fit for the migration guide, which should be opinionated about what changes are important and which should give you advice on how to update your code.

@JordanMartinez
Copy link
Contributor Author

It may be that we should create a separate document that aggregates changelogs across all libraries (which is how this content reads). That seems valuable to me.

I was thinking the same thing when I saw how long the guide becomes when this gets added. It has value, but I agree with you that the main goal of the migration guide is to tell people how to fix broken code.

I was wondering if perhaps there should be a folder for the migration guide storing two files. The first file being the current migration guide. The second being the ecosystem update release notes.

@thomashoneyman
Copy link
Member

thomashoneyman commented Mar 26, 2022

I was thinking the same thing when I saw how long the guide becomes when this gets added. It has value, but I agree with you that the main goal of the migration guide is to tell people how to fix broken code.

Yea. I think we should use the previous migration guide as the template for this migration guide:
https://github.com/purescript/documentation/blob/master/migration-guides/0.14-Migration-Guide.md

I was wondering if perhaps there should be a folder for the migration guide storing two files. The first file being the current migration guide. The second being the ecosystem update release notes.

Yea, we could have say a 0.15-Migration-Guide.md and 0.15-Ecosystem-Changelog.md or something like that. How are you getting these changelogs appended together? It would be nice to have that saved somewhere if we're going to start doing this every year.

@JordanMartinez
Copy link
Contributor Author

Note to self about math: purescript/package-sets#1075 (comment)

@JordanMartinez
Copy link
Contributor Author

I've updated this PR to just include the full unabbreviated release notes as a separate file.

I'll open a separate PR that focuses on how to actually update broken code.

Comment on lines 9 to 11
New features:

Bugfixes:
Copy link
Member

@thomashoneyman thomashoneyman Mar 30, 2022

Choose a reason for hiding this comment

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

I think we should strip out empty sections. Could this be done easily in your script by trimming out sections that make it to the next section header without any contents besides newlines?

Copy link
Member

Choose a reason for hiding this comment

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

For example, this section would read:

### purescript-arrays

Breaking changes:
- Migrate FFI to ES modules (#218 by @kl0tl and @JordanMartinez)
- Drop deprecated `group'` and `empty` (#219 by @JordanMartinez)

Other improvements:
- Fixed minor documentation issue with `find` (#216 by @JamieBallingall)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could this be done easily in your script by trimming out sections that make it to the next section header without any contents besides newlines?

It should be possible so long as all repos are using the same general headers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

I also tried removing a lot of duplicate changelog entries and then removing entire libraries if that's all the changes that were made there. The latest commit shows this diff.

Duplicate breakage being:
- migrate FFI to ES modules
- update project/deps to PS 0.15
- drop `math`; update imports
- drop MonadZero instances

Duplicate changes being:
- Added purs-tidy formatter
- other improvements irrelevant to non-maintainers
Copy link
Member

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

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

Fantastic! Thank you!

Co-authored-by: Thomas Honeyman <hello@thomashoneyman.com>
@JordanMartinez JordanMartinez changed the title Add all changelog entries for breaking changes Add all changelog entries for breaking changes in PureScript 0.15.0 Mar 30, 2022
@JordanMartinez JordanMartinez merged commit 939dd1e into purescript:master Mar 30, 2022
@JordanMartinez JordanMartinez deleted the add-changelog-release-notes branch March 30, 2022 22:04
@JordanMartinez
Copy link
Contributor Author

Note: there are still the affjax changelog entries. If there are other breaking changes added later, it'll be easy to update this.

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

Successfully merging this pull request may close these issues.

3 participants