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

doc: fix "test-internet.yml" workflows #40177

Closed
wants to merge 1 commit into from
Closed

doc: fix "test-internet.yml" workflows #40177

wants to merge 1 commit into from

Conversation

suryapratapsinghsuryavanshi
Copy link
Contributor

No description provided.

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Sep 22, 2021
@tniessen
Copy link
Member

Thank you @suryapratapsinghsuryavanshi!

@suryapratapsinghsuryavanshi
Copy link
Contributor Author

Welcome, @tniessen :)

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

I'm always amazed that the only way to know if a workflow file is valid is to ship it and wait for GitHub to try and execute it...

@targos targos added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 22, 2021
@github-actions
Copy link
Contributor

Fast-track has been requested by @targos. Please 👍 to approve.

@suryapratapsinghsuryavanshi
Copy link
Contributor Author

Yeah, but there is another way that we should use an IDE which checks whether our YAML file is valid or not because most of the time the indentation is in the wrong places.

@Mesteery
Copy link
Contributor

Shouldn't the subsystem be build: instead of doc:?

@Trott
Copy link
Member

Trott commented Sep 22, 2021

Shouldn't the subsystem be build: instead of doc:?

Yes. I'll fix that on landing. Thanks for flagging it.

@Trott
Copy link
Member

Trott commented Sep 22, 2021

I'm always amazed that the only way to know if a workflow file is valid is to ship it and wait for GitHub to try and execute it...

At least for this one, we shouldn't have to wait because it can be started manually...

Trott pushed a commit that referenced this pull request Sep 22, 2021
PR-URL: #40177
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Trott commented Sep 22, 2021

Landed in c7da13c.

Thanks for the contribution! 🎉

@Trott Trott closed this Sep 22, 2021
@Trott
Copy link
Member

Trott commented Sep 22, 2021

Looks like it works now. (I haven't waited for the tests to pass, but the fact that they're even running is farther than it was getting before!) https://github.com/nodejs/node/runs/3681508986?check_suite_focus=true

Thanks for the fix!

@suryapratapsinghsuryavanshi
Copy link
Contributor Author

@Trott, Your Welcome :)

targos pushed a commit that referenced this pull request Oct 4, 2021
PR-URL: #40177
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants