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

[web] Cockpit starter-kit webpack sync #413

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Conversation

dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Jan 25, 2023

Problem

Not a problem, but a suggestion: to keep our webpack config file in sync with the Cockpit Starter Kit counterpart as much as possible.

At this moment, we're three commits behind, namely:

Solution

To apply them

Pending

Not sure if the second one (build: don't move pkg/lib/ to src/lib/) proceeds here

Testing

  • Tested manually

martinpitt and others added 2 commits January 25, 2023 15:29
By default, webpack does not clean up dist/, so when you e.g. touch the
code and rebuild with `NODE_ENV=devel`, the old compressed assets from
a previous `production` build were left in place, and cockpit could
serve the old wrong ones. Enable the `clean` output option to fix this [1].

Also force the rewrite of all files with `compareBeforeEmit: false`, so
that the time stamps actually get updated on a webpack run. This fixes
`make` rules and unnecessary rebuilds when nothing changed.

Fixes #563

[1] https://webpack.js.org/configuration/output/#outputclean
Include CSS linting by default for our CSS/SCSS files, originally
introduced in cockpit. The stylelint configuration is copied from
760a4628282e02fbcef3.
gap: var(--spacer-small);
}

section > svg {
grid-aread: icon;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice linter catch around here 😅

@coveralls
Copy link

coveralls commented Jan 25, 2023

Coverage Status

Coverage: 75.758%. Remained the same when pulling cf31c19 on sync-starter-kit-webpack into 92c9d90 on master.

@imobachgs
Copy link
Member

It is a good idea to keep this stuff in sync. IMHO, we should add the style linter to our CI workflow.

@dgdavid
Copy link
Contributor Author

dgdavid commented Jan 26, 2023

[...] IMHO, we should add the style linter to our CI workflow.

Done. Thanks for remembering it.

@dgdavid
Copy link
Contributor Author

dgdavid commented Jan 26, 2023

It is a good idea to keep this stuff in sync

Agree. But what do you think about cockpit-project/starter-kit@b3a9565? Does it look needed for you?

@dgdavid dgdavid changed the title Sync starter kit webpack [web] Cockpit starter-kit webpack sync Jan 26, 2023
@dgdavid dgdavid marked this pull request as ready for review February 7, 2023 15:45
@dgdavid dgdavid merged commit 143dbc0 into master Feb 7, 2023
@dgdavid dgdavid deleted the sync-starter-kit-webpack branch February 7, 2023 15:45
@imobachgs imobachgs mentioned this pull request Feb 13, 2023
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.

None yet

5 participants