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

Move CMP CLI test output files to $BLDTOP/test-runs/test_cmp_cli/ #12934

Closed
wants to merge 2 commits into from
Closed

Move CMP CLI test output files to $BLDTOP/test-runs/test_cmp_cli/ #12934

wants to merge 2 commits into from

Conversation

DDvO
Copy link
Contributor

@DDvO DDvO commented Sep 21, 2020

This is the real fix for the issue addressed by #12925.

  • tests are added or updated

@DDvO DDvO mentioned this pull request Sep 21, 2020
Closed
2 tasks
@DDvO
Copy link
Contributor Author

DDvO commented Sep 23, 2020

@levitte, I've addressed your comments and all tests pass.
Anything else to do, or can this be approved now?

@DDvO
Copy link
Contributor Author

DDvO commented Sep 26, 2020 •
edited
Loading

Ping for OTC approval

@mattcaswell mattcaswell mentioned this pull request Sep 28, 2020
Closed
@DDvO
Copy link
Contributor Author

DDvO commented Oct 1, 2020

@levitte, can you approve this now?

@DDvO
Copy link
Contributor Author

DDvO commented Oct 5, 2020

@levitte, also this PR is waiting for further handling/approval since nearly two weeks meanwhile..

@t8m t8m added the branch: master Merge to master branch label Oct 5, 2020
t8m
t8m approved these changes Oct 5, 2020
View reviewed changes
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

LGTM

@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: otc review pending labels Oct 5, 2020
@levitte
Copy link
Member

levitte commented Oct 6, 2020

@levitte, also this PR is waiting for further handling/approval since nearly two weeks meanwhile..

Sorry, I thought I had already done that... too many other distractions, I guess

@DDvO DDvO mentioned this pull request Oct 6, 2020
Closed
1 task
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@levitte levitte added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 6, 2020
@levitte
Copy link
Member

levitte commented Oct 6, 2020

Merge it

openssl-machine pushed a commit that referenced this pull request Oct 6, 2020
@DDvO
Test.pm: Add result_dir and export both result_dir and result_file
bd60ac4 
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #12934)
openssl-machine pushed a commit that referenced this pull request Oct 6, 2020
@DDvO
Move CMP CLI test output files to BLDTOP/test-runs/test_cmp_cli/
c5fc675 
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #12934)
@DDvO
Copy link
Contributor Author

DDvO commented Oct 6, 2020

Merged - thanks @levitte for your comments and @t8m for the approval.

@DDvO DDvO closed this Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@levitte levitte levitte left review comments

@t8m t8m t8m approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DDvO @levitte @openssl-machine @t8m