Skip to content

Conversation

@mgol
Copy link
Member

@mgol mgol commented Feb 24, 2020

Changes:

  1. Test on Node.js 12 instead of 10
  2. Make Release.walk an async function, await each provided method. This
    makes it possible to easier integrate with asynchronous flows in various steps.

@mgol
Copy link
Member Author

mgol commented Feb 24, 2020

@timmywil I will need this for the Inquirer prompting to provide a jQuery blog post link.

@mgol mgol requested a review from arschmitz February 24, 2020 21:16
@mgol mgol assigned mgol and unassigned timmywil and arschmitz Feb 24, 2020
… to 12

Changes:
1. Test on Node.js 12 instead of 10
2. Make `Release.walk` an async function, `await` each provided method. This
makes it possible to easier integrate with asynchronous flows in various steps.
3. Throw on unhandled rejections, making jquery-release quit if any of the async
steps passed to `Release.walk` ends up in a rejected state.
@mgol
Copy link
Member Author

mgol commented Feb 28, 2020

@timmywil I backported unhandled rejection handling here, can you have another look?

The diff between what you reviewed and the current version: https://github.com/jquery/jquery-release/compare/632a6fc665ce6cd81d92e591108005cdcb00dcc6..25a1b9d206a91146b36be9747171140fd56ec256

Copy link
Member

@arschmitz arschmitz left a comment

Choose a reason for hiding this comment

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

This looks good to me much prefer async functions was going to ask for the error handling when i first looked this morning but looks like you added it so i'm good

@mgol mgol changed the title Build:util: await methods passed to Release.walk, switch from Node 10 to 12 Build:util: await methods passed to Release.walk, test on Node 12 Mar 2, 2020
@mgol mgol merged commit ff3a435 into jquery:master Mar 2, 2020
@mgol mgol deleted the refactor branch March 2, 2020 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants