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

frontend: allow config-based build-chroot tags #2834

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

praiskup
Copy link
Member

@praiskup praiskup commented Aug 5, 2023

Copy link
Member

@FrostyX FrostyX left a comment

Choose a reason for hiding this comment

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

Just some minor things, otherwise looks good.
Thank you.

# regexp pattern is matched against BuildChroot fully qualified names in the
# format "user/project:dirname/chroot_name/pkgname", so example rule might
# look like
# "pattern": "r"john/projectX.*/fedora-.*-x86_64/chromium",
Copy link
Member

Choose a reason for hiding this comment

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

nit: There shouldn't be " before r"..."

"""
if "EXTRA_BUILDCHROOT_TAGS_COMPILED" in app.config:
return
new_array = app.config["_EXTRA_BUILDCHROOT_TAGS_COMPILED"] = []
Copy link
Member

Choose a reason for hiding this comment

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

I'd drop the new_array and access _EXTRA_BUILDCHROOT_TAGS_COMPILED directly.

"""
Given the build_id, finish the build with succeeded state
Given the build_id, finish the source build (but don't import)
Copy link
Member

Choose a reason for hiding this comment

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

finish_srpm_and_import but finish the source build (but don't import). I am confused 😄

That is a duplicate info to the 'down_revision' field, and we often
forget to update when rebasing against other revisions.
@praiskup
Copy link
Member Author

praiskup commented Aug 8, 2023

Thank you for the review, updated!

praiskup added a commit to praiskup/copr that referenced this pull request Aug 8, 2023
@praiskup praiskup requested a review from FrostyX August 8, 2023 14:21
@praiskup praiskup merged commit 9e71174 into fedora-copr:main Aug 9, 2023
7 checks passed
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