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

Update "tested on" badge in README to include PHP v8.3 #514

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

fredden
Copy link
Member

@fredden fredden commented Sep 13, 2023

While this change is hopefully useful, the primary purpose of this pull request is to test if the recent changes to the Coveralls workflow work well with an external contributor or not.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Thanks @fredden for submitting this PR. Approving it for the content of the PR. Now 🤞🏻 to see if the upload to Coveralls works... ;-)

@jrfnl jrfnl added this to the 1.0.x Next milestone Sep 13, 2023
@fredden
Copy link
Member Author

fredden commented Sep 13, 2023

@jrfnl
Copy link
Member

jrfnl commented Sep 13, 2023

@fredden It has indeed. I've reported back to Coveralls with our test results.

If you don't mind, I'm going to leave this PR open for the time being and may ask you to rebase once or twice after I make changes to the workflow to see if the changes have the desired effect. Will wait first for a response from the Coveralls team though.

I suspect that removing the github-token input will fix this, but that is completely counter to the Coveralls docs + counter to the info in the error message for the failed workflow....

@jrfnl
Copy link
Member

jrfnl commented Sep 13, 2023

Still for testing purposes, I've removed the Coveralls token now (PR #515). I suspect if you rebase, the build will pass.

(Note: I still won't merge the PR (yet), even when it does pass, as we may need to do more testing and having an open PR from a fork is useful for that)

@fredden
Copy link
Member Author

fredden commented Sep 14, 2023

🎉 all (required) checks passed!

@jrfnl
Copy link
Member

jrfnl commented Sep 15, 2023

@fredden Thanks for the rebase and happy to see it works!

I've done some extensive testing with a private repo now as well and written up my test outcomes. I'll wait for a reply from the Coveralls team, but if no more tests are needed, I'll merge this PR over the next few days.

Thanks for your help testing this!

@jrfnl
Copy link
Member

jrfnl commented Nov 24, 2023

@fredden I've finally got the go-ahead from Coveralls, so shouldn't need this PR anymore for further testing. Could you rebase the PR to get passed the merge conflict ? I'll merge it after that.

@fredden
Copy link
Member Author

fredden commented Nov 24, 2023

I've rebased this pull request. It looks like PHP 8.3 was already added to the list in duplicate in 64ee4ae

@jrfnl
Copy link
Member

jrfnl commented Nov 24, 2023

I've rebased this pull request.

Thanks for the rebase. Not sure what you mean by ?

It looks like PHP 8.3 was already added to the list in duplicate in 64ee4ae

You mean in the badge ? Yes, that's 100% true. I updated the badge as the build is no longer allowed to fail. Sorry, I forgot that was included in this PR too. The badge ordering though was not yet done, so this PR should still be a valid change.

@jrfnl jrfnl merged commit f76bab4 into PHPCSStandards:develop Nov 24, 2023
47 checks passed
@fredden fredden deleted the badges branch November 24, 2023 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants