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

Release v0.4.4 #253

Merged
merged 1 commit into from
May 17, 2023
Merged

Release v0.4.4 #253

merged 1 commit into from
May 17, 2023

Conversation

12wrigja
Copy link
Contributor

@12wrigja 12wrigja commented May 2, 2023

There is quite a lot to review here, and there's a couple more PRs that we should merge (and I'll update the release notes to include).

I wasn't quite sure where to put many of the changes in this release - observable changes in the order functions were called or properties were accessed ended up being something I skipped unless it sounded particularly important (e.g. sorted field access on property bags), but if those sorts of changes should be mentioned I'll have to take another pass over the commits to include the (many) relevant ones.

Similarly, there were lots of changes that are bug fixes, breaking changes, possibly both at the same time? Happy to reorganize the lists as people see fit - I don't have a good sense for whether fixing some of these bugs is likely to be a breaking change.

@12wrigja 12wrigja requested a review from justingrant May 10, 2023 23:43
@12wrigja
Copy link
Contributor Author

Based on some offline feedback I've pruned/rewritten parts of the release notes to bundle together the various larger changes in this release.

I'd particularly appreciate feedback in deciding if some of the smaller items are worth mentioning. I've tried to remove things I think are really edge-case (observability / iteration ordering changes), but I've kept things I think might produce visible errors.

Copy link
Contributor

@justingrant justingrant left a comment

Choose a reason for hiding this comment

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

Wow, amazing work on this. I made a bunch of minor clarifying suggestions, and applied some general changes:

  • Put methods and class names in backticks
  • Put Temporal. in front of all class names
  • Replaced "timezone" with "time zone".

I made so many minor edits that I'm sure that some of them are wrong, so I'll probably want to take another review pass on this after these changes are included.

Thanks again for doing this!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@justingrant
Copy link
Contributor

BTW, looks like the linter is complaining about something.

@12wrigja
Copy link
Contributor Author

Thanks so much for all the feedback!

I've incorporated all this feedback plus some small adjustments to guidance on how to handle the ZDT/DateTimeFormat interaction changes. PTAL.

Copy link
Contributor

@justingrant justingrant left a comment

Choose a reason for hiding this comment

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

Looks great! Only a few minor suggestions this time. Thanks for putting in so much work to get this done... our users will be grateful too!

Do you want to go ahead and publish the release after this is merged? Or are there other upstream PRs that we should wait to merge?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Philip Chimento <[email protected]>
Co-authored-by: Justin Grant <[email protected]>
@12wrigja
Copy link
Contributor Author

I'm going to merge and create the release - I don't see any particularly interesting changes upstream that we need to pull in for this release in particular.

@12wrigja 12wrigja merged commit 73ed89b into js-temporal:main May 17, 2023
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