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

Only clone once, not once per subdir #443

Merged
merged 7 commits into from
Sep 24, 2023
Merged

Only clone once, not once per subdir #443

merged 7 commits into from
Sep 24, 2023

Conversation

ericphanson
Copy link
Member

@ericphanson ericphanson commented Nov 17, 2022

Closes #442

This PR has two commits (+ formatting); the first lets tests pass locally on my machine, by specifying the init-branch instead of relying on it being set to master. The second does the change to close #442. Besides speeding things up and possibly avoiding weird cloning issues, this also makes the code more "compositional" and simplifies the tests.

edit: I've also pushed an additional commit to re-use the same local clone for each PR. It seems wasteful to make so many clones, and in #442 for some reason for me the first clone seems to work and the subsequent ones fail.

@ericphanson
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Nov 17, 2022
@bors
Copy link
Contributor

bors bot commented Nov 17, 2022

try

Build failed:

@ericphanson
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Nov 17, 2022
@ericphanson
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Nov 17, 2022
@lgoettgens
Copy link
Contributor

What is the state of this? I would like to tackle #408, but having this PR here as a baseline would make things a lot easier.

@ericphanson
Copy link
Member Author

Just need review iirc

@lgoettgens
Copy link
Contributor

maybe ping them again? After 10 months I find that reasonable

@ericphanson
Copy link
Member Author

If I know my GitHub notifications these comments will do it :). Also I have merge rights so if you or anyone else wants to review, that would suffice for me to feel comfortable merging.

@DilumAluthge
Copy link
Member

@fchorney or @mattBrzezinski Could you review this?

@fchorney
Copy link
Collaborator

Hey sorry, I have changed jobs this summer and am no longer working in Julia. I probably won't really have time to look at stuff like this going forward.

@mattBrzezinski
Copy link
Member

@fchorney or @mattBrzezinski Could you review this?

I'll put it on my TODO list, hopefully get around to it in a few days. Just busy with other things currently :(

@lgoettgens
Copy link
Contributor

lgoettgens commented Sep 21, 2023

The docs failure is already resolved by #453. This needs a rebase now.

@ericphanson
Copy link
Member Author

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 24, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit e1ce1c4 into master Sep 24, 2023
6 checks passed
@bors bors bot deleted the eph/clone-once branch September 24, 2023 10:29
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.

Why clone once per subdir?
5 participants