Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Require PRs from pulp_file and pulp-smash #3435

Merged
merged 1 commit into from Apr 16, 2018
Merged

Conversation

dkliban
Copy link
Member

@dkliban dkliban commented Apr 13, 2018

Require PRs from pulp_file and pulp-smash

The PRs listed in this commit message are just for demonstrating that
Travis will checkout those PRs when testing the PR that contains this
commit.

closes #3530
https://pulp.plan.io/issues/3530

Required PR: pulp/pulp_file#65
Required PR: pulp/pulp-smash#770

@dkliban dkliban force-pushed the require-pr branch 8 times, most recently from 3382953 to 0d11011 Compare April 15, 2018 11:33
@@ -8,26 +8,34 @@ pushd common/ && pip install -e . && popd
pushd pulpcore/ && pip install -e . && popd
pushd plugin/ && pip install -e . && popd

export COMMIT_MSG=$(git log --format=%B -n 1 HEAD)
export COMMIT_MSG=$(git show HEAD^2 -s)
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, there's a variable for this I believe. Something like $TRAVIS_COMMIT_MESSAGE.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is, but it's for the merge commit of the PR into the branch it's going to. So it only says "Merge of blah into bleh". I have to find the parent commit of that.

Copy link
Contributor

@dralley dralley Apr 16, 2018

Choose a reason for hiding this comment

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

git log has an option for ignoring merge commits --no-merges. You could try that.

@dkliban dkliban changed the title [WIP] Require PRs from pulp_file and pulp-smash Require PRs from pulp_file and pulp-smash Apr 16, 2018
cd pulp-smash
git fetch origin +refs/pull/$PULP_SMASH_PR_NUMBER/merge
git checkout $PULP_SMASH_SHA
pip install -e .
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we should DRY this up with a bash function?

Copy link
Member Author

Choose a reason for hiding this comment

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

let's DRY up in a separate PR. please?

.. _file: https://github.com/PulpQE/pulp-smash/tree/master/pulp_smash/tests/pulp3/file
.. _pulp-smash: https://github.com/PulpQE/pulp-smash/

Continuous Integration
Copy link
Member

Choose a reason for hiding this comment

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

I think these docs are great, thanks!

@dkliban dkliban force-pushed the require-pr branch 6 times, most recently from 2c6dc12 to 52c1589 Compare April 16, 2018 17:08
The PRs listed in this commit message are just for demonstrating that
Travis will checkout those PRs when testing the PR that contains this
commit.

closes pulp#3530
https://pulp.plan.io/issues/3530

Required PR: pulp/pulp_file#65
Required PR: pulp/pulp-smash#770
@daviddavis
Copy link
Contributor

Should the commit be updated to remove the lines Required PR: ...?

Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

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

One question. Other than that, LGTM.

@dkliban
Copy link
Member Author

dkliban commented Apr 16, 2018

The commit message says that those lines are in the commit message for testing purposes only. Should I remove them and test that the PR still passes tests?

@daviddavis
Copy link
Contributor

No, I missed that. Let's merge.

@dkliban dkliban merged commit b4d0932 into pulp:3.0-dev Apr 16, 2018
@dkliban dkliban deleted the require-pr branch April 16, 2018 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants