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

Replace Travis with Github Actions #46

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ShawnCZek
Copy link
Contributor

@ShawnCZek ShawnCZek commented Jan 28, 2023

As requested in #45 (comment), I have replaced Travis with GitHub Actions.
The actions are inspired by other official OAuth 2.0 Client providers.

To make the code coverage work, you have to set up Codecov integration for this repository. Once you do that, update the badge in README as well, please, as I cannot handle this. Otherwise, all other actions should work out of the box.

@ShawnCZek
Copy link
Contributor Author

ShawnCZek commented Jan 28, 2023

Also, two other things should be considered before merging this:

  1. What is the .scrutinizer.yml file for? It looks like the integration is not even configured for this repository.
  2. Should we not check the code style against PSR-12 instead of the outdated PSR-2?

Though, when I am thinking of it now, these should be resolved in a different PR.

@wohali
Copy link
Owner

wohali commented Jan 28, 2023

1. What is the `.scrutinizer.yml` file for? It looks like the integration is not even configured for this repository.

I think I had this set up locally via a pre-commit hook. We could add those to the repo if you like.

2. Should we not check the code style against PSR-12 instead of the outdated PSR-2?

PSR-12 (July 2019) was not even created when this project was born (October 2017). I'm open to updating coding standards if appropriate.

Both changes belong in a new PR, but they could be in the same PR structured as 2 commits (all intermediate changes squashed, please).

I hope to look at this PR tomorrow.

@ShawnCZek
Copy link
Contributor Author

@wohali I have no experience with Scrutinizer. I would say that is up to you to either set it up or remove it from the repository.

We can switch to PSR-12 in a different PR so this PR brings only one change.

@ShawnCZek
Copy link
Contributor Author

@wohali, hey, have you had any luck reviewing this PR? Or are there any other changes I should make? 👀

@wohali
Copy link
Owner

wohali commented Sep 25, 2023

I have not, but I promise I am still here. I will get to this eventually... sorry for the delay.

@ShawnCZek
Copy link
Contributor Author

@wohali Hey, do you have any update on this or other PRs?

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

Successfully merging this pull request may close these issues.

2 participants