Skip to content

Fix unexpected input Warning message during release build/test process#437

Merged
felixarntz merged 13 commits intotrunkfrom
fix/388-unexpected-warning
Aug 31, 2022
Merged

Fix unexpected input Warning message during release build/test process#437
felixarntz merged 13 commits intotrunkfrom
fix/388-unexpected-warning

Conversation

@mukeshpanchal27
Copy link
Copy Markdown
Member

@mukeshpanchal27 mukeshpanchal27 commented Jul 18, 2022

Summary

The PR updates the versions of actions and removes the Read .nvmrc task and uses node-version-file: '.nvmrc' for the version number. I referred Gutenberg End-to-End Tests GitHub workflow.

Fixes #388

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@mukeshpanchal27 mukeshpanchal27 added [Type] Bug An existing feature is broken Infrastructure Issues for the overall performance plugin infrastructure Needs Testing no milestone PRs that do not have a defined milestone for release labels Jul 18, 2022
peterwilsoncc
peterwilsoncc approved these changes Jul 20, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

Looks good to me.

The code appears to match the code in the Gutenberg workflow.

The modified action appears to be passing too https://github.com/WordPress/performance/runs/7388754887?check_suite_focus=true

mehulkaklotar
mehulkaklotar reviewed Jul 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@mehulkaklotar mehulkaklotar left a comment

Choose a reason for hiding this comment

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

@mukeshpanchal27 did you try to update to actions/checkout@v3 and actions/setup-node@v3? There is update to the library that we need to test and check if that fixes the notices or not. https://github.com/actions/setup-node#usage

@mukeshpanchal27
Copy link
Copy Markdown
Member Author

mukeshpanchal27 commented Jul 20, 2022 •
edited
Loading

did you try to update to actions/checkout@v3 and actions/setup-node@v3?

If I update to actions/checkout@v3 and actions/setup-node@v3, it works perfectly. In the current approach, I refer to Gutenberg workflow.

@mukeshpanchal27
Copy link
Copy Markdown
Member Author

mukeshpanchal27 commented Jul 21, 2022

Do we need to updated actions/checkout@v3 and actions/setup-node@v3? Additionally, with these changes can we update the node version to 16.x if we prefer the latest version?

@felixarntz and @adamsilverstein, I would be interested to hear your thoughts on this?

@mukeshpanchal27 mukeshpanchal27 self-assigned this Jul 22, 2022
@adamsilverstein
Copy link
Copy Markdown
Member

adamsilverstein commented Aug 2, 2022 •
edited
Loading

Do we need to updated actions/checkout@v3 and actions/setup-node@v3? Additionally, with these changes can we update the node version to 16.x if we prefer the latest version?

+1 to updating both the action and node version, especially if this resolves the original Warning message. @mukeshpanchal27 - Please go ahead and do that and I'll give this another test.

adamsilverstein
adamsilverstein reviewed Aug 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

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

looking good! will test once final change in place

@mukeshpanchal27 mukeshpanchal27 force-pushed the fix/388-unexpected-warning branch from bffa655 to 6164fbe Compare August 3, 2022 06:36
@akshitsethi akshitsethi mentioned this pull request Aug 3, 2022
Closed
3 tasks
@mukeshpanchal27
Copy link
Copy Markdown
Member Author

mukeshpanchal27 commented Aug 3, 2022

@adamsilverstein Can you please re-review it? Thank you!

adamsilverstein
adamsilverstein approved these changes Aug 16, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for fixing this.

felixarntz
felixarntz reviewed Aug 23, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@mukeshpanchal27 Mostly LGTM, I have two follow-up questions.

eugene-manuilov
eugene-manuilov reviewed Aug 26, 2022
View reviewed changes
eugene-manuilov
eugene-manuilov approved these changes Aug 31, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

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

LGTM

felixarntz
felixarntz approved these changes Aug 31, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thank you @mukeshpanchal27!

@felixarntz felixarntz merged commit cd17fc7 into trunk Aug 31, 2022
@felixarntz felixarntz deleted the fix/388-unexpected-warning branch August 31, 2022 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eugene-manuilov eugene-manuilov eugene-manuilov approved these changes

@adamsilverstein adamsilverstein adamsilverstein approved these changes

@felixarntz felixarntz felixarntz approved these changes

@jjgrainger jjgrainger Awaiting requested review from jjgrainger

@JustinyAhin JustinyAhin Awaiting requested review from JustinyAhin

@mehulkaklotar mehulkaklotar Awaiting requested review from mehulkaklotar

+1 more reviewer

@peterwilsoncc peterwilsoncc peterwilsoncc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mukeshpanchal27 mukeshpanchal27

Labels

Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Bug An existing feature is broken

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate unexpected input Warning message during release build/test process

8 participants

@mukeshpanchal27 @adamsilverstein @peterwilsoncc @eugene-manuilov @felixarntz @mehulkaklotar @mxbclang @akshitsethi