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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

New CI #1883

Closed
paulmelnikow opened this issue Feb 10, 2020 · 17 comments
Closed

New CI #1883

paulmelnikow opened this issue Feb 10, 2020 · 17 comments

Comments

@paulmelnikow
Copy link
Member

It's cool that we're getting things going with GitHub Actions! Thanks so much to @merlinnot for the legwork (and patience!) 馃弲 and @gr2m for bringing your expertise on this.

I'm trying to get a grasp on the current status of things. I'm seeing some builds which are intermittently failing. For example, on https://github.com/nock/nock/actions/runs/36768497, Node 8 failed on Windows, and on the commit that ran just before https://github.com/nock/nock/actions/runs/36768146, Node 12 failed on Windows. It seems the goal of #1881 is to try to address this, though I'm not sure I understand why an npm upgrade would help in Node 12.

Also, we should update the required checks. Right now Travis is still required, but does not run on pull requests, so that definitely should be removed. Which of the other builders are stable? Should we make them all required except for Windows, to start?

@paulmelnikow
Copy link
Member Author

To start, I removed Travis from the required checks and added the linter, formatter, and Ubuntu test builds. I also turned on "Require branches to be up to date" for all protected branches (it was only on for master).

@paulmelnikow
Copy link
Member Author

Seems a bit better since #1878 was merged, though I'm still seeing some transient failures, e.g. https://github.com/nock/nock/pull/1825/checks?check_run_id=439376631

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Feb 16, 2020

@gr2m
Copy link
Member

gr2m commented Feb 16, 2020

It fails because there are no changes to commit. I've added a | true to just ignore errors in that case

@gr2m
Copy link
Member

gr2m commented Feb 16, 2020

Let me fix that real quick, if anyone has a better idea we can iterate on it

@gr2m
Copy link
Member

gr2m commented Feb 16, 2020

@paulmelnikow
Copy link
Member Author

The Prettier autofix seems to be working now, which is cool!

Unfortunately for some reason the checks don't run on the new commit. I've seen this happen twice in the last couple hours:

Screen Shot 2020-02-18 at 3 54 56 PM

@merlinnot
Copy link
Contributor

merlinnot commented Feb 18, 2020

This is a know issue with GitHub Actions. Any changes made by GitHub Actions (using the GITHUB_TOKEN) don't trigger any events. They did that so that you can't have infinite loops, while limiting usability for power users. The only way to circumvent the issue is to use a Personal Access Token.

Although to be perfectly fair, it did happen in the past that Prettier had unstable formatting, in that case we'd actually have an infinite loop.

@gr2m
Copy link
Member

gr2m commented Feb 18, 2020

we can use a personal access token from @nockbot for things like that instead.

Although to be perfectly fair, it did happen in the past that Prettier had unstable formatting, in that case we'd actually have an infinite loop.

Can you explain? I'm not sure how this would end up in an infinite loop in this case? After the styling was fixed, there shouldn't be another commit fixing it again

@merlinnot
Copy link
Contributor

"unstable" means that Prettier will format its own output in a different way. Here is a list of the past issues of that kind: https://github.com/prettier/prettier/issues?q=is%3Aissue+is%3Aopen+unstable+label%3Aarea%3Aidempotency. As you can see, it happens quite often.

If this happens, after formatting and committing, the new execution would format files in a different way, then commit again, then another execution would be triggered, format files in yet another way, ...

@gr2m
Copy link
Member

gr2m commented Feb 18, 2020

Ohh I see, so it would get back-and-forth, for example?

We could add a check on our side to make sure that the format action does not triggered by a commit coming from the same action?

@merlinnot
Copy link
Contributor

Yes, we absolutely can! Good idea. I believe we can add an if condition like this: github.event.commits[0].message != 'style: format files with Prettier'.

@merlinnot
Copy link
Contributor

Although I'm not sure if that's the right path for pull_request triggers, I only used it for push. It might be different.

@gr2m
Copy link
Member

gr2m commented Feb 18, 2020

here is a list of the different events and their payloads https://developer.github.com/v3/activity/events/types/

@stale
Copy link

stale bot commented May 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.

@stale stale bot added the stale label May 19, 2020
@gr2m gr2m added the pinned label May 19, 2020
@stale stale bot removed the stale label May 19, 2020
@mastermatt
Copy link
Member

@gr2m can this issue be considered complete and closed?

@mastermatt mastermatt removed the pinned label Aug 9, 2020
@gr2m
Copy link
Member

gr2m commented Aug 9, 2020

I think so. We can create new issues for specific problems with the CI, ping me if I can help

@gr2m gr2m closed this as completed Aug 9, 2020
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

No branches or pull requests

4 participants