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

Allow passing user and branch to downstream #22

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

alexfikl
Copy link
Collaborator

@alexfikl alexfikl commented Apr 28, 2022

Hacked this in here to be able to use (see inducer/pymbolic#74)

test_downstream https://github.com/alexfikl/loopy.git@equality-mapper

and have it pick up another repo/branch. Any way to test that this doesn't break the world? (Tried it locally and it was nice enough to rewrite my gitconfig, but it did clone things correctly)

Suggestions for handling this more gracefully very welcome!

@inducer
Copy link
Owner

inducer commented Apr 28, 2022

Tried it locally and it was nice enough to rewrite my gitconfig, but it did clone things correctly

You're welcome! duck :)

Any way to test that this doesn't break the world?

Sadly, no. I've wished for that myself, too.

ci-support.sh Outdated
Comment on lines 712 to 717
url="https://github.com/$user_name/$proj_name.git"
if [[ "$proj_name" = "mirgecom" ]]; then
git clone "https://github.com/illinois-ceesd/$proj_name.git"
default_url="https://github.com/illinois-ceesd/$proj_name.git"
else
git clone "https://github.com/inducer/$proj_name.git"
default_url="https://github.com/inducer/$proj_name.git"
fi
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm. I know what was here before wasn't beautiful, but this logic seems like one step too far. I also know that I'm more likely to say this because it's not me committing the crime.

Realistically, I think the better answer is to permit passing in a pip-style git URL: git+https://...@branchname. That ought to be easy enough to pick apart?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not easy at all! Bash string slicing and dicing is horrible 😨

@alexfikl alexfikl force-pushed the extend-downstream branch from 360fda1 to d514ccc Compare April 28, 2022 20:43
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

LGTM! (And 🎉 much nicer! Thanks!) Let's see how it does in inducer/pymbolic#74, and if that comes back clean, then it's good to go in.

@alexfikl
Copy link
Collaborator Author

alexfikl commented Apr 28, 2022

LGTM! (And tada much nicer! Thanks!) Let's see how it does in inducer/pymbolic#74, and if that comes back clean, then it's good to go in.

Yep! Seems to be working, the failures this far are from the EqualityMapper itself, from what I can tell 🎉

EDIT: The loopy downstream seems to be passing now (the others need work).

@inducer inducer merged commit f680353 into inducer:main Apr 28, 2022
@inducer
Copy link
Owner

inducer commented Apr 28, 2022

Thx!

@alexfikl alexfikl deleted the extend-downstream branch May 1, 2022 18:46
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