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

Sugarcoat resources #51

Merged
merged 4 commits into from
Jan 6, 2022
Merged

Sugarcoat resources #51

merged 4 commits into from
Jan 6, 2022

Conversation

ShivanKaul
Copy link
Collaborator

No description provided.

@@ -1,8 +1,26 @@
[
{
"name": "brave-fix",
"aliases": ["bf"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we need this alias?

Copy link
Collaborator

Choose a reason for hiding this comment

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

antonok-edm
antonok-edm previously approved these changes Jan 3, 2022
Copy link
Collaborator

@antonok-edm antonok-edm left a comment

Choose a reason for hiding this comment

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

Looks good to me but let's confirm with @ryanbr about the bf alias before merging

@ryanbr
Copy link
Collaborator

ryanbr commented Jan 4, 2022

Yeah we still need "bf" @ShivanKaul

@ShivanKaul
Copy link
Collaborator Author

@ryanbr gotcha, added that back in. It would be nice if we could generate the alias automatically; I don't want to retrofit an alias for every sugarcoat resource just because of brave-fix though, so leaving it as a manual change, added a note to generateMetadataJsonFromScriptResources.js that a manual fixup is needed after running it.

antonok-edm
antonok-edm previously approved these changes Jan 5, 2022
@ShivanKaul ShivanKaul merged commit 09c9b6f into master Jan 6, 2022
@ShivanKaul ShivanKaul deleted the local-sugarcoat-resources branch January 6, 2022 15:50
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