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.

Sign up for GitHub

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

Jump to bottom

Don't create duplicate issues when configuration is invalid #1075

Merged
merged 1 commit into from
Apr 29, 2021
Merged

Don't create duplicate issues when configuration is invalid #1075

merged 1 commit into from
Apr 29, 2021

Conversation

csomh
Copy link
Contributor

@csomh csomh commented Apr 29, 2021 •
edited
Loading

Kudos to @ppisar for reporting this 👨‍🎤

Signed-off-by: Hunor Csomortáni csomh@redhat.com

jpopelka
jpopelka approved these changes Apr 29, 2021
View reviewed changes
Copy link
Member

@jpopelka jpopelka left a comment

Choose a reason for hiding this comment

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

good catch!

@csomh
Copy link
Contributor Author

csomh commented Apr 29, 2021

Please hold the horses! I would like to add some tests.

mfocko
mfocko approved these changes Apr 29, 2021
View reviewed changes
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.

damn that suffix idea... 🙃

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@csomh
Copy link
Contributor Author

csomh commented Apr 29, 2021

One more round of review please! Here is what changed:

  • Changed the way create_issue_if_needed works. There is a more explicit title arg now, which should simplify things and the only thing that is added in the method is a [packit] prefix.
  • Added a few unit tests for this method, just to be sure 😜

@csomh csomh requested review from jpopelka and mfocko April 29, 2021 15:03
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

mfocko
mfocko approved these changes Apr 29, 2021
View reviewed changes
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 👍

tests/unit/test_config.py Outdated Show resolved Hide resolved
Don't create duplicate issues when configuration is invalid
c6432bb 
Signed-off-by: Hunor Csomortáni <csomh@redhat.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@ppisar ppisar mentioned this pull request Apr 29, 2021
Closed
@csomh csomh added the mergeit When set, zuul wil gate and merge the PR. label Apr 29, 2021
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 89eab9e into packit:main Apr 29, 2021
@csomh csomh deleted the dont-spam branch April 29, 2021 16:03
@lachmanfrantisek
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfocko mfocko mfocko approved these changes

@jpopelka jpopelka Awaiting requested review from jpopelka

Assignees
No one assigned
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@csomh @lachmanfrantisek @jpopelka @mfocko