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

[Merged by Bors] - refactor: tweak and move functionality for getting all modules out of scripts #13339

Closed
wants to merge 3 commits into from

Conversation

grunweg
Copy link
Collaborator

@grunweg grunweg commented May 29, 2024

With #13245, two scripts in scripts would like to use very similar information:
move it to a central place instead (a file in Mathlib/Util). Suggestions for better locations are welcome!

Also, split the getAll function in two pieces, getting the file names and transforming this into module names.
While #11853 only requires the latter, #13245 needs the former (which is computed anyway).
Only accessible the converted module names and converting them back to files would be absurd.


Open in Gitpod

@grunweg grunweg added awaiting-review CI Modifies the continuous integration / deployment setup labels May 29, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) label May 29, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) label May 29, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

This PR/issue depends on:

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label May 29, 2024
@grunweg grunweg removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) CI Modifies the continuous integration / deployment setup labels May 29, 2024
… scripts/

It is needed in other scripts: the easiest way to depend on this is to move this to mathlib. Better places are welcome!
Mathlib/Util/GetAllModules.lean Outdated Show resolved Hide resolved
@adomani
Copy link
Collaborator

adomani commented Jun 1, 2024

This looks good to me! The rename of the variable should have really happened when I refactored the code, but I only noticed it now.

Thanks!

(I won't be at a computer until Monday, so I can't really test this code right now.)

@grunweg
Copy link
Collaborator Author

grunweg commented Jun 1, 2024

For the future, resolveDefaultRootModules in leanprover-community/batteries#811seems related, but different.

@kim-em
Copy link
Contributor

kim-em commented Jun 1, 2024

bors d+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jun 1, 2024

✌️ grunweg can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@grunweg
Copy link
Collaborator Author

grunweg commented Jun 1, 2024

Thanks for the quick review
bors r+

mathlib-bors bot pushed a commit that referenced this pull request Jun 1, 2024
… `scripts` (#13339)

With #13245, two scripts in `scripts` would like to use very similar information:
move it to a central place instead (a file in `Mathlib/Util`). Suggestions for better locations are welcome!

Also, split the `getAll` function in two pieces, getting the file names and transforming this into module names.
While #11853 only requires the latter, #13245 needs the former (which is computed anyway).
Only accessible the converted module names and converting them back to files would be absurd.

- [x] depends on: #11853 which adds these utility functions
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jun 1, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: tweak and move functionality for getting all modules out of scripts [Merged by Bors] - refactor: tweak and move functionality for getting all modules out of scripts Jun 1, 2024
@mathlib-bors mathlib-bors bot closed this Jun 1, 2024
@mathlib-bors mathlib-bors bot deleted the MR-extract-module-utils branch June 1, 2024 13:49
callesonne pushed a commit that referenced this pull request Jun 4, 2024
… `scripts` (#13339)

With #13245, two scripts in `scripts` would like to use very similar information:
move it to a central place instead (a file in `Mathlib/Util`). Suggestions for better locations are welcome!

Also, split the `getAll` function in two pieces, getting the file names and transforming this into module names.
While #11853 only requires the latter, #13245 needs the former (which is computed anyway).
Only accessible the converted module names and converting them back to files would be absurd.

- [x] depends on: #11853 which adds these utility functions
grunweg added a commit that referenced this pull request Jun 7, 2024
… `scripts` (#13339)

With #13245, two scripts in `scripts` would like to use very similar information:
move it to a central place instead (a file in `Mathlib/Util`). Suggestions for better locations are welcome!

Also, split the `getAll` function in two pieces, getting the file names and transforming this into module names.
While #11853 only requires the latter, #13245 needs the former (which is computed anyway).
Only accessible the converted module names and converting them back to files would be absurd.

- [x] depends on: #11853 which adds these utility functions
mattrobball added a commit that referenced this pull request Jun 12, 2024
mattrobball added a commit that referenced this pull request Jun 12, 2024
AntoineChambert-Loir pushed a commit that referenced this pull request Jun 20, 2024
… `scripts` (#13339)

With #13245, two scripts in `scripts` would like to use very similar information:
move it to a central place instead (a file in `Mathlib/Util`). Suggestions for better locations are welcome!

Also, split the `getAll` function in two pieces, getting the file names and transforming this into module names.
While #11853 only requires the latter, #13245 needs the former (which is computed anyway).
Only accessible the converted module names and converting them back to files would be absurd.

- [x] depends on: #11853 which adds these utility functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants