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

Name all the recipes #4858

Merged
merged 82 commits into from May 18, 2022
Merged

Name all the recipes #4858

merged 82 commits into from May 18, 2022

Conversation

Quezler
Copy link
Contributor

@Quezler Quezler commented May 17, 2022

The :kjs_ recipe ids are hard to track down, this pull request will get rid of all of them.

Using the CI script i have tracked down all recipes that lack a custom id, at first i tried to fix all of them by hand but that proved pretty much impossible due to avoiding duplicate name conflicts in unification scripts, as well as deciding a name.

In this pull request the easy/small ones are given a prefix & name by hand, but for the larger bulk i have introduced a helper function called md5, which basically takes the part from after :kjs_and puts the supplied id_prefix in front of it.

This will help to find where the random ids are defined, but avoids the effort required for coming up with naming schemes.

Therefore this is a compromise, ideally we'd have all recipes named by hand, but these are at least a little easier to maintain since the id points to the right file (& function) responsible for adding the recipe.

Once this is merged one simply has to search for md5 in the kubejs scripts to find which recipes still require a manual name, this pull request is not the end goal but merely a stepping stone.

Before i'll turn draft mode off i'll go over it using the github checkboxes to clean it up a little more, but after that it should be good to go.

I have checked that no added recipes got lost by comparing the added recipes file of the ci of the base & pull branch:

const fs = require('fs');

let fork = JSON.parse(fs.readFileSync('added_fork.json'));
let origin = JSON.parse(fs.readFileSync('added_origin.json'));

console.log([
  Object.entries(fork).length,
  Object.entries(origin).length,
]);

Strangely this returns [ 20403, 20402 ], so no recipes got lost but one slipped in, should be no cause for concern. 🤔
Screen Shot 2022-05-17 at 11 55 21

Quezler added 30 commits May 16, 2022 19:48
@Quezler Quezler changed the title Name all the (expert) recipes Name all the recipes May 17, 2022
@Quezler
Copy link
Contributor Author

Quezler commented May 17, 2022

all kjs_ recipes in both modes have been either named manually or prefixed with md5_ instead:
Screen Shot 2022-05-17 at 13 02 51

@Quezler Quezler marked this pull request as ready for review May 17, 2022 11:06
@NielsPilgaard
Copy link
Collaborator

Ready to merge, or do you want to wait for a potential bug fix in Botany Crops?

@Quezler
Copy link
Contributor Author

Quezler commented May 18, 2022

I have exempted botany pots from the fallback id so we don't have to wait on it for now ^-^

@NielsPilgaard NielsPilgaard merged commit 2c21ca7 into EnigmaticaModpacks:develop May 18, 2022
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.

None yet

2 participants