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

Do not repeat custom copr restriction message #1716

Merged

Conversation

FrNecas
Copy link

@FrNecas FrNecas commented Oct 19, 2022

Some slight refactoring of duplicate message check into status reporter using an optional argument + then using it in appropriate places.

TODO:

  • Write new tests or update the old ones to cover new functionality.
  • Update doc-strings where appropriate.

Fixes #1686

RELEASE NOTES BEGIN
Packit now won't repeatedly comment in pull requests about the need to migrate configuration of allowed forge projects to Copr.
RELEASE NOTES END

@softwarefactory-project-zuul

This comment was marked as outdated.

@jpopelka
Copy link
Member

The failing openshift tests are my fault, sorry, I'm working on the fix.

packit_service/constants.py Outdated Show resolved Hide resolved
packit_service/worker/reporting.py Outdated Show resolved Hide resolved
@softwarefactory-project-zuul

This comment was marked as outdated.

@FrNecas FrNecas force-pushed the frnecas-reporting-duplicates branch from 61e0536 to c46e73e Compare October 19, 2022 14:14
@softwarefactory-project-zuul

This comment was marked as outdated.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ pre-commit SUCCESS in 1m 42s
✔️ packit-service-tests SUCCESS in 1m 53s
✔️ packit-service-tests-openshift SUCCESS in 13m 31s

Copy link
Member

@mfocko mfocko left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -393,6 +393,7 @@ def run(self) -> TaskResults:
status_reporter = StatusReporter.get_instance(
project=self.project,
commit_sha=self.data.commit_sha,
packit_user=self.service_config.get_github_account_name(),
Copy link
Member

Choose a reason for hiding this comment

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

Could you please check what is the account name? IIRC it might be prefixed with app/, but on GitLab, we »do not« have that prefix.

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure if I follow what you mean by the app/ prefix. The method just returns one of packit-as-a-service[bot], packit-as-a-service-stg[bot], packit-as-a-service-dev[bot] depending on the deployment used. This should match the commenter name in the Github PRs/commits

packit_service/worker/reporting.py Outdated Show resolved Hide resolved
Comment on lines +36 to +44
class DuplicateCheckMode(Enum):
"""Enum of possible behaviour for handling duplicates when commenting."""

# Do not check for duplicates
do_not_check = auto()
# Check only last comment from us for duplicate
check_last_comment = auto()
# Check the whole comment list for duplicate
check_all_comments = auto()
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if we want to switch between the modes and don't be consistent but I like it after all..;) really nicely implemented!

softwarefactory-project-zuul bot added a commit to packit/ogr that referenced this pull request Oct 24, 2022
Deprecate CommitComment.comment in favour of body

Related to packit/packit-service#1716
RELEASE NOTES BEGIN
CommitComment.comment has been deprecated in favour of CommitComment.body to make the naming consistent across objects.
RELEASE NOTES END

Reviewed-by: Laura Barcziová <None>
Reviewed-by: None <None>
@FrNecas FrNecas force-pushed the frnecas-reporting-duplicates branch from c46e73e to f3557eb Compare October 24, 2022 08:03
@FrNecas
Copy link
Author

FrNecas commented Oct 24, 2022

Updated the access to commit's comment body to match packit/ogr#748 .

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

pre-commit POST_FAILURE in 1m 46s
✔️ packit-service-tests SUCCESS in 1m 54s
✔️ packit-service-tests-openshift SUCCESS in 11m 20s

@FrNecas
Copy link
Author

FrNecas commented Oct 24, 2022

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ pre-commit SUCCESS in 1m 53s
✔️ packit-service-tests SUCCESS in 2m 03s
✔️ packit-service-tests-openshift SUCCESS in 10m 57s

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

nicely done!

@FrNecas FrNecas added the mergeit When set, zuul wil gate and merge the PR. label Oct 24, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ pre-commit SUCCESS in 1m 46s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit da94cc9 into packit:main Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restricting custom copr build message written multiple times
5 participants