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

Consolidate Aliases, formula_renames.json, cask_renames.json and tap_migrations.json into single file. #16614

Closed
1 task done
reitermarkus opened this issue Feb 7, 2024 · 6 comments
Labels
discussion Input solicited from others features New features help wanted We want help addressing this outdated PR was locked due to age

Comments

@reitermarkus
Copy link
Member

reitermarkus commented Feb 7, 2024

Verification

Provide a detailed description of the proposed feature

All of these are handled by Formulary or CaskLoader, so it makes sense to merge all of these into a single file, e.g migrations.json.

Aliases and renames are handled almost identically in code, so it is kind of weird that one is loaded from a directory structure while the other is loaded from a JSON file. An alias is basically a rename without a warning.

What is the motivation for the feature?

Simplify code used for loading formulae/casks.

How will the feature be relevant to at least 90% of Homebrew users?

Easier management of aliases/renames/migrations. Only relevant to maintainers: Allows simplifying the code for handling all these.

What alternatives to the feature have been considered?

Alternatively, only get rid of Aliases in favour of a JSON file so that at least we don't have different ways of loading similar data.

@reitermarkus reitermarkus added features New features discussion Input solicited from others labels Feb 7, 2024
@apainintheneck
Copy link
Contributor

There's already some work to combine those as a part of #16410. That project started off with trying to speed up JSON loading and then became about JSON file size (smaller files can be loaded faster) and finally we've started the process to create a new JSON format. The plan is for each JSON file to represent a tap so all of the tap migrations and renames will be folded into the same file.

Only one PR has been merged so far and it handles creating the file for formulae but the loader code is something I need to work on next. We're currently generating and signing the file on CI but there is still the possibility of changes and/or us deciding that any network/performance improvements are not worth the added complexity. So early days, we'll see how it goes.

@MikeMcQuaid
Copy link
Member

All of these are handled by Formulary or CaskLoader, so it makes sense to merge all of these into a single file, e.g migrations.json.

This seems like a nice idea. The main issue I see is that archived repositories aren't going to update these files probably ever so we'll need to keep two code paths around forever.

The plan is for each JSON file to represent a tap so all of the tap migrations and renames will be folded into the same file.

Yeh, this feels like the best way to handle this 👍🏻. For most users: they won't have these taps tapped at all so won't even use this code path.

How will the feature be relevant to at least 90% of Homebrew users?

N/A

Please fill this out, thanks.

@reitermarkus
Copy link
Member Author

we'll need to keep two code paths around forever

Why would archived repos be exempt from deprecation forever?

@MikeMcQuaid
Copy link
Member

@reitermarkus It is nice to not break existing repos migrations.

Copy link

github-actions bot commented Mar 6, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Mar 6, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2024
@MikeMcQuaid MikeMcQuaid added help wanted We want help addressing this and removed stale No recent activity labels Mar 14, 2024
@MikeMcQuaid
Copy link
Member

Keeping this open but let's scope it purely to the new internal JSON API.

@github-actions github-actions bot added the outdated PR was locked due to age label Apr 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion Input solicited from others features New features help wanted We want help addressing this outdated PR was locked due to age
Projects
None yet
Development

No branches or pull requests

3 participants