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

[build targets] use targets both from build job and test job #996

Merged
merged 2 commits into from
Feb 26, 2021
Merged

[build targets] use targets both from build job and test job #996

merged 2 commits into from
Feb 26, 2021

Conversation

lbarcziova
Copy link
Member

@lbarcziova lbarcziova commented Feb 24, 2021 •
edited
Loading

When we have configuration like this:

jobs:
- job: copr_build
  trigger: pull_request
  metadata:
    targets:
    - fedora-stable-x86_64

- job: tests
  trigger: pull_request
  metadata:
    targets:
    - fedora-rawhide-x86_64

build for targets both from build job and test job (currently the build would not run for fedora-rawhide-x86_64).

The tests were testing only mocked functionality, so there are some changes. They now test the configured_build_targets and configured_test_targets instead of the build_targets and test_targets which call get_valid_build_targets which uses Copr and Bodhi, so now we check the targets used as input for get_valid_build_targets.

Related to #976

@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@lbarcziova

This comment has been minimized.

Sign in to view
mfocko
mfocko reviewed Feb 24, 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.

otherwise LGTM :)

packit_service/worker/build/build_helper.py Show resolved Hide resolved
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
lachmanfrantisek
lachmanfrantisek approved these changes Feb 25, 2021
View reviewed changes
Copy link
Member

@lachmanfrantisek lachmanfrantisek left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me. And it's also simplified -- nice!

@lbarcziova

This comment has been minimized.

Sign in to view
@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@jpopelka
Copy link
Member

rebase please, CI should be fixed

@softwarefactory-project-zuul

This comment has been minimized.

Sign in to view
@lbarcziova lbarcziova force-pushed the build-targets-for-tests branch from 90d81ca to 76d7e8a Compare February 25, 2021 16:25
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@lbarcziova lbarcziova added the mergeit When set, zuul wil gate and merge the PR. label Feb 25, 2021
@TomasTomecek
Copy link
Member

Awesome, another UX bug fixed!

@softwarefactory-project-zuul
Copy link
Contributor

Build failed (gate pipeline).

@lbarcziova lbarcziova added mergeit When set, zuul wil gate and merge the PR. and removed mergeit When set, zuul wil gate and merge the PR. labels Feb 25, 2021
@softwarefactory-project-zuul
Copy link
Contributor

Build failed (gate pipeline).

@lbarcziova
Copy link
Member Author

CI apparently doesn't like this PR 😣

@mfocko
Copy link
Member

mfocko commented Feb 25, 2021

CI apparently doesn't like this PR 😣

it's just telling you to start the extended weekend sooner 🙃

@mfocko mfocko added mergeit When set, zuul wil gate and merge the PR. and removed mergeit When set, zuul wil gate and merge the PR. labels Feb 25, 2021
@softwarefactory-project-zuul
Copy link
Contributor

Build failed (gate pipeline).

@jpopelka jpopelka added mergeit When set, zuul wil gate and merge the PR. and removed mergeit When set, zuul wil gate and merge the PR. labels Feb 26, 2021
@softwarefactory-project-zuul
Copy link
Contributor

Build failed (gate pipeline).

@jpopelka jpopelka merged commit af32599 into packit:main Feb 26, 2021
@lbarcziova lbarcziova deleted the build-targets-for-tests branch March 4, 2021 15:37
@mfocko mfocko mentioned this pull request Mar 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfocko mfocko mfocko left review comments

@TomasTomecek TomasTomecek TomasTomecek approved these changes

@lachmanfrantisek lachmanfrantisek lachmanfrantisek approved these changes

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.
5 participants
@lbarcziova @jpopelka @TomasTomecek @mfocko @lachmanfrantisek