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

FIX Add missing repo from hardcoded consts #37

Merged

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Nov 14, 2022

This repo was missing in #36 because it's not in the cowpat.

I've also added a commit here which will help avoid similar mistakes in the future by calling out if repositories are missing which were present in the previous release.

IMPORTANT

The person who merges this should tag a new patch version immediately afterward

Parent issue

@michalkleiner
Copy link

What if we ever wanted to remove a repo, we will get a warning until the last two sets have the same entries again? Not an urgent thing for now, may be never needed, just that we don't have a clear way of handling that.

Copy link

@michalkleiner michalkleiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't test the warning mechanism, the consts file addition is legit. Merge at your convenience.

@GuySartorelli
Copy link
Member Author

What if we ever wanted to remove a repo, we will get a warning until the last two sets have the same entries again?

Running hardcoded.php is a manual process we do at beta-time. We currently manually copy and paste the output into the array in consts.php. So we'll only get the warning for the one release when it's manually run, and if we're intentionally removing the module from recipes then we just don't include the module we got warned about and therefore it won't warn again next time we run the script.

hardcoded.php Outdated
if (!empty($missing)) {
$formatColor = "\033[31m";
$endFormat = "\033[0m";
echo "\n" . $formatColor . 'Warning: The following modules were in the previous release but are missing in this release:' . $endFormat . "\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
echo "\n" . $formatColor . 'Warning: The following modules were in the previous release but are missing in this release:' . $endFormat . "\n";
echo "\n" . $formatColor . 'Warning: The following modules were in the last release in INSTALLER_TO_REPO_MINOR_VERSIONS but are missing from .cow.pat.json:' . $endFormat . "\n";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it really obvious where the issue is

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@emteknetnz emteknetnz merged commit 728bef5 into silverstripe:1.5 Nov 14, 2022
@emteknetnz emteknetnz deleted the pulls/1.5/add-missing-repo branch November 14, 2022 21:47
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.

3 participants