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

Upgrade node #1117

Draft
wants to merge 33 commits into
base: develop
Choose a base branch
from
Draft

Upgrade node #1117

wants to merge 33 commits into from

Conversation

will-byrne
Copy link

No description provided.

.nvmrc Outdated
lts/gallium
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: newline

@nlisgo
Copy link
Member

nlisgo commented Jun 8, 2022

I've tested all steps in the current readme. All worked well with some adjustments to Makefile and readme.

There was one test that failed for me:

it('dispatches a chapterChanged event on the supplied element',

Will create a ticket to add a make step to allow individual tests to be run but that is not blocking.

Biggest concern is the length of time it takes when running make watch and re-rendering the assets. A potential improvement would be to only render the sass if changes detected there rather than re-rendering all assets if any kind of asset changes.

Great work.

We will adapt the Jenkins pipelines as a followup to this work so will maintain the docker compose steps until then.

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