gorilla / mux

Package gorilla/mux is a powerful HTTP router and URL matcher for building Go web servers with 🦍
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
20.93k stars 1.85k forks source link

regexp: use iota instead of hardcoded values for regexType* #679

Closed michaelgrigoryan25 closed 2 years ago

michaelgrigoryan25 commented 2 years ago

Summary of Changes

Replaced hardcoded constant regexType* declarations with iota.

michaelgrigoryan25 commented 2 years ago

I noticed that some other PRs are also left out by CircleCI. Maybe someone needs to approve the run, before it appears here.

amustaque97 commented 2 years ago

I noticed that some other PRs are also left out by CircleCI. Maybe someone needs to approve the run, before it appears here.

No I don't think so. Even when I raise any PR CircleCI runs automatically without anyone's approval.

I was going through this https://discuss.circleci.com/t/circle-ci-builds-do-not-trigger-on-pull-requests-of-some-particular-users/29788/2

Could you also amend the last commit and re-push your changes.

Thank you

michaelgrigoryan25 commented 2 years ago

I see. I've amended my latest commit and re-pushed it (without any changes of course), but still don't see anything happening from the Circle CI side.

michaelgrigoryan25 commented 2 years ago

Hey @amustaque97, any updates on this?

amustaque97 commented 2 years ago

Hey @amustaque97, any updates on this?

thanks for the ping. I was able to test changes locally with latest golang version. Changes look good to me. I would label these changes as "Janitorial".

I will try to spend sometime on CI part to understand why is it not triggering for some of PRs? Why emphasising on the CI part to make sure it doesn't break anything in the different golang environments.

amustaque97 commented 2 years ago

@michaelgrigoryan25 could you please make any minor commit and push it here? Don't force-push, just a regular commit push.

michaelgrigoryan25 commented 2 years ago

Nope, still nothing. Perhaps you should update the Circle CI runner configuration file, to track PRs alongside regular commits?

amustaque97 commented 2 years ago

Nope, still nothing. Perhaps you should update the Circle CI runner configuration file, to track PRs alongside regular commits?

I'm checking with the Circle CI team. According to Circle CI, a new commit should trigger the pipeline. I have pinged them with the current PR. let's wait for their reply till tomorrow.

image

I hope that should be ok?

michaelgrigoryan25 commented 2 years ago

I see, so I suppose there's nothing wrong with the current configuration?

amustaque97 commented 2 years ago

I see, so I suppose there's nothing wrong with the current configuration?

Yes! At least, I was not able to find any issues with the current setup.

amustaque97 commented 2 years ago

@elithrar I had a chat with the CricleCI support team regarding CI not triggering. They checked in their logs and asked the owner of the project to re-authenticate Circle CI. There are many replies back-forth, not possible to share all of them here. I would request you to please follow the link as mentioned in the screenshot.

image

Would it be possible to try a reauthentication using the following guide here.

amustaque97 commented 2 years ago

@michaelgrigoryan25 once reauthentication is done. We can cross-check the pipeline by reverting the README.md commit. Later Owner of the project will merge the PR based on their availability.

michaelgrigoryan25 commented 2 years ago

Makes sense, thanks for the follow up @amustaque97.

elithrar commented 2 years ago

It's not triggering because it's coming from a forked PR, no?

michaelgrigoryan25 commented 2 years ago

Thanks for the reply @elithrar. I don't really know at this point.

amustaque97 commented 2 years ago

It's not triggering because it's coming from a forked PR, no?

@elithrar :wave:, I don't think so. The strange part is that let us take my PR: https://github.com/gorilla/mux/pull/678 CI is working all time. This is another forked PR https://github.com/gorilla/mux/pull/681 CI is working fine.

CI is not working for this PR: https://github.com/gorilla/mux/pull/675, https://github.com/gorilla/mux/pull/674, https://github.com/gorilla/mux/pull/676

I checked all the closed/merged PRs https://github.com/gorilla/mux/pulls?q=is%3Apr+is%3Aclosed CI ran for all the PRs and most of them are forked PRs.

elithrar commented 2 years ago

It's not consistent though: some newer PRs are clearly being checked against CI.

Is this not related to the ongoing GitHub webhook incident?

michaelgrigoryan25 commented 2 years ago

I think so. The issues have been resolved only 20 hours ago. https://www.githubstatus.com/incidents/gvzcw6sd0xxb

amustaque97 commented 2 years ago

I think so. The issues have been resolved only 20 hours ago. https://www.githubstatus.com/incidents/gvzcw6sd0xxb

Since it is resolved now. You can revert the change of the README.md file and we can check now.

I would be happy to be wrong here. I still feel CI will not trigger. 😅

michaelgrigoryan25 commented 2 years ago

Yeah, you are right @amustaque97. I just reverted the commit but nothing initiated.

amustaque97 commented 2 years ago

Yeah, you are right @amustaque97. I just reverted the commit but nothing was initiated.

@elithrar, I can confirm now it is not related to Github webhook Incident :)

michaelgrigoryan25 commented 2 years ago

Can't someone just run the CI directly? Seems to big of an issue for such a small PR.

elithrar commented 2 years ago

There’s no way to fix CI for this PR. I agree it is overkill for the change here.

Merging.

michaelgrigoryan25 commented 2 years ago

Thanks for the follow-up @elithrar, and I think I found the issue:

git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

https://app.circleci.com/pipelines/github/gorilla/mux/142/workflows/677527ce-5484-42b1-aeaa-f7ffb81802d1/jobs/931/parallel-runs/0/steps/0-101.

Perhaps you are using the old authentication method (passwords) for GitHub. I'm not 100% sure, but I hope this info will help.

elithrar commented 2 years ago

I re-authed, reinstalled their app, and later PRs are being run. There is a small group of PRs that never got created and CircleCI doesn't backfill.

(GitHub's webhook issue was longer running than the public notice said)

elithrar commented 2 years ago

(That error is because I forced a re-auth - that's for the main branch, not for this PR, which never appears on CircleCI's side)

elithrar commented 2 years ago

See passing CI on main - https://app.circleci.com/pipelines/github/gorilla/mux/142/workflows/8611d32d-0399-4393-832e-a0bcec0da4bc

Note that pull/679 never populated. Not much we can do until it happens again. C'est la vie.

image
michaelgrigoryan25 commented 2 years ago

Cool!