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

Renames main.js to app.js #2341

Merged
merged 3 commits into from
Sep 30, 2020
Merged

Renames main.js to app.js #2341

merged 3 commits into from
Sep 30, 2020

Conversation

roblarsen
Copy link
Member

closes #2340

@roblarsen roblarsen requested a review from coliff September 24, 2020 23:18
@roblarsen roblarsen self-assigned this Sep 24, 2020
@coliff
Copy link
Member

coliff commented Sep 28, 2020

looks good Rob- I think you need to run gulp build so that the empty 'app.js' is in the dist directory. (and main.js deleted).

@roblarsen
Copy link
Member Author

@coliff 👀

@roblarsen roblarsen mentioned this pull request Sep 28, 2020
@coliff
Copy link
Member

coliff commented Sep 29, 2020

when I run gulp build it updates Modernizr to 3.11.3. it's updated to that version in package.json so it should really be the same in dist/ too.

I think we should set dependabot to ignore Modernizr updates - that should be updated manually as it requires a new build of H5Bp to be made. Otherwise we get issues like this... I'll look into that.

@roblarsen
Copy link
Member Author

Yeah, the latest modernizr is included here. We should look into it so that it's manual and not depdendabot driven, for sure. But this one was never updated.

Copy link
Contributor

@sthiepaan sthiepaan left a comment

Choose a reason for hiding this comment

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

Looks promising! Please have a look at the suggested change(s). Thank you! ✌️

src/doc/usage.md Show resolved Hide resolved
@sthiepaan
Copy link
Contributor

sthiepaan commented Sep 29, 2020

@coliff dependabot have ignored_updates option which should solve that problem. The downside of doing this is that we need to be aware of bumping ignored package(s) manually 🤷‍♂️

Btw. do we need Modernizr at all? Isn't polyfill.io itself enough? Here is a list of supported browsers. Oh wait, there is no polyfill.io in index.html at all, just in documentation 🙈

@roblarsen
Copy link
Member Author

@sthiepaan Since Modernizr is actually shipped with this project, I already watch it actively.

@roblarsen
Copy link
Member Author

I'll merge this tonight (GMT -04:00)

@roblarsen roblarsen merged commit 8f0adb5 into master Sep 30, 2020
@coliff coliff deleted the 2340-rename-main-js branch September 30, 2020 03:17
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.

Rename main.js -> app.js + update documentation
3 participants