Skip to content

frontend: Do not extract comments in separate files on release. #156

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
merged 1 commit into from
Sep 9, 2019

Conversation

La0
Copy link
Contributor

@La0 La0 commented Sep 9, 2019

To avoid building an extra file with just comments that crashes the deployment task (no content type detected)

@La0 La0 requested a review from marco-c September 9, 2019 12:14
@La0 La0 self-assigned this Sep 9, 2019
@marco-c
Copy link
Collaborator

marco-c commented Sep 9, 2019

The nice thing about having a separate file with LICENSE info is that we don't load it at runtime (even though right now the file is pretty small).
Could we instead avoid the crash in the deployment task itself?

@La0
Copy link
Contributor Author

La0 commented Sep 9, 2019

I filed an issue on task-boot to do that mozilla/task-boot#61

But i think this is not really interesting for the frontend for now, as it only avoid loading the mustache header

@marco-c
Copy link
Collaborator

marco-c commented Sep 9, 2019

Yeah right now it's not so useful, but if we add more files with license comments it might be a win.

Copy link
Collaborator

@marco-c marco-c left a comment

Choose a reason for hiding this comment

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

Fine by me if you want to merge this as-is and file an issue to revert it after the task-boot issue is fixed, or if you want to fix the issue in task-boot and just close this.

@La0 La0 merged commit 8c5de7b into mozilla:master Sep 9, 2019
@La0 La0 deleted the fix-frontend-build branch September 9, 2019 13:01
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.

2 participants