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

Issue/592 #598

Merged
merged 12 commits into from
Nov 22, 2021
Merged

Issue/592 #598

merged 12 commits into from
Nov 22, 2021

Conversation

bapcltd-marv
Copy link
Contributor

re: #592

generating code coverage should be possible, although personally I switched our fork to codecov a while ago.

Copy link
Collaborator

@Sebbo94BY Sebbo94BY left a comment

Choose a reason for hiding this comment

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

Would be also cool to integrate the respective badge in the README. :)

[![CI](https://github.com/barbushin/php-imap/actions/workflows/php.yml/badge.svg)](https://github.com/barbushin/php-imap/actions/workflows/php.yml)

.github/workflows/php.yml Show resolved Hide resolved
.github/workflows/php.yml Outdated Show resolved Hide resolved
.github/workflows/php.yml Show resolved Hide resolved
@bapcltd-marv
Copy link
Contributor Author

This PR now integrates changes that would close #593 and #599

running pinned version per
barbushin#598 (comment)
whilst also running against latest version as heads up for any potential
issues
This was referenced May 27, 2021
@bapcltd-marv
Copy link
Contributor Author

@Sebi94nbg should travis be dropped as part of this PR?

@Sebbo94BY
Copy link
Collaborator

@Sebi94nbg should travis be dropped as part of this PR?

If both are doing the same, I would say yes as we otherwise would only need to manage the same thing twice.

@bapcltd-marv
Copy link
Contributor Author

@Sebi94nbg should travis be dropped as part of this PR?

If both are doing the same, I would say yes as we otherwise would only need to manage the same thing twice.

The only issue at present is this PR isn't set up to generate coverage reports, I'll give the codeclimate action a go without using a hardcoded reporter ID.

@bapcltd-marv
Copy link
Contributor Author

@Sebi94nbg should travis be dropped as part of this PR?

If both are doing the same, I would say yes as we otherwise would only need to manage the same thing twice.

The only issue at present is this PR isn't set up to generate coverage reports, I'll give the codeclimate action a go without using a hardcoded reporter ID.

per 993b51b, travis has been dropped.
per 0746e30, the reporter ID should be added as a repo secret, to make it easier to manage forks.

@Sebbo94BY
Copy link
Collaborator

Alright. Can I merge it now? Or are you still changing something on it? :D

@bapcltd-marv
Copy link
Contributor Author

Alright. Can I merge it now? Or are you still changing something on it? :D

  • the code climate reporter ID needs to be added to the repo secrets via CC_TEST_REPORTER_ID
  • OR the commit that removed the hardcoded value needs to be reverted
  • the branch configs need to be cleaned up after merging so they no longer trigger mostly just for this branch
  • the "drop php 7.2" issues need to be closed
  • the code style issues referenced in Using deprecated php-cs-fixer methods #594 need to be resolved so the CI action passes (outside the scope of my current dayjob responsibilities)

@Sebbo94BY
Copy link
Collaborator

  • the code climate reporter ID needs to be added to the repo secrets via CC_TEST_REPORTER_ID

@barbushin can you please set the secret / environment variable in the repository settings? You're the only one, who has the permissions to do this. :)

Otherwise we need to keep the secret in Git, what is not that beautiful.

@bapcltd-marv
Copy link
Contributor Author

Otherwise we need to keep the secret in Git, what is not that beautiful.

it's more a matter of making it easier to manage coverage in forks.

Should I keep my Test Reporter ID secret?
Your repo's test coverage ID is only used to identify your repo when submitting test coverage payloads. It can't be used to access any of your data.

https://docs.codeclimate.com/docs/finding-your-test-coverage-token

aragon999 pushed a commit to aragon999/php-imap that referenced this pull request Jun 13, 2021
running pinned version per
barbushin#598 (comment)
whilst also running against latest version as heads up for any potential
issues
aragon999 pushed a commit to aragon999/php-imap that referenced this pull request Jun 13, 2021
aragon999 pushed a commit to aragon999/php-imap that referenced this pull request Jun 13, 2021
@Sebbo94BY
Copy link
Collaborator

I'll merge this now in order to fix the cs-fixer issues in the pipeline.

@Sebbo94BY Sebbo94BY merged commit 8a23291 into barbushin:develop Nov 22, 2021
Sebbo94BY pushed a commit that referenced this pull request Dec 14, 2021
running pinned version per
#598 (comment)
whilst also running against latest version as heads up for any potential
issues
Sebbo94BY pushed a commit that referenced this pull request Dec 14, 2021
Sebbo94BY pushed a commit that referenced this pull request Dec 14, 2021
dropping travis as discussed in #598 (comment)
Sebbo94BY pushed a commit that referenced this pull request Jan 6, 2022
Travis CI integration has been removed/dropped in May 2021. See commit 993b51b.
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.

None yet

2 participants