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

Make sure #26968 is not unfixed after #35707 #35832

Closed
wants to merge 13 commits into from

Conversation

tornaria
Copy link
Contributor

Calling initialize-runtime-globals will run set-pathnames and be subject
to the issue described in #26968.

Thus the workaround introduced in #35195 has to be done before anything
that may call set-pathnames (e.g. initialize-runtime-globals).

⌛ Dependencies

@tornaria
Copy link
Contributor Author

@vbraun @antonio-rojas please make sure this is included when #35707 is merged.

@github-actions
Copy link

Documentation preview for this PR (built with commit b742776; changes) is ready! 🎉

@antonio-rojas
Copy link
Contributor

@vbraun @antonio-rojas please make sure this is included when #35707 is merged.

wouldn't it make more sense to make a PR on #35707's branch?

@tornaria
Copy link
Contributor Author

@vbraun @antonio-rojas please make sure this is included when #35707 is merged.

wouldn't it make more sense to make a PR on #35707's branch?

Indeed, but I still find this a bit unnatural. a PR on #35707 branch is associated to your fork of sage, but it's not associated to sage main repo. E.g. workflows in my PR to your repo won't run (see antonio-rojas#2).

Also, there's no notice in #35707 about my PR to your PR...

As an alternative, you can always fetch my branch and replace yours in #35707 making this PR superfluous (and I think you can close this PR).

@tornaria
Copy link
Contributor Author

Closed as this is now included in #35707 .

@tornaria tornaria closed this Jun 25, 2023
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