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

move CSS build to webpack #9983

Merged
merged 2 commits into from
Jan 28, 2020
Merged

move CSS build to webpack #9983

merged 2 commits into from
Jan 28, 2020

Conversation

silverwind
Copy link
Member

  • added new 'make webpack' target
  • deprecated 'make js' and 'make css'
  • extend webpack config to load the less files
  • updated docs

I had to rename the source file of arc-green.less to avoid generating a useless JS entrypoint via webpack-fix-style-only-entries which would not work with different source/destination filenames. I hear that there should be cleaner solutions possible once we upgrade to Webpack 5.

@silverwind silverwind force-pushed the webpack-css branch 2 times, most recently from eba151b to 3278869 Compare January 25, 2020 18:45
@lafriks lafriks added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Jan 25, 2020
@silverwind
Copy link
Member Author

Partially unrelated, but I reworked the "Building Gitea" in CONTRIBUTING.md to just link to the hacking guide as info in it was partially outdated.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 25, 2020
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 25, 2020
@lafriks lafriks added this to the 1.12.0 milestone Jan 25, 2020
@codecov-io
Copy link

codecov-io commented Jan 25, 2020

Codecov Report

Merging #9983 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9983      +/-   ##
==========================================
- Coverage   42.25%   42.25%   -0.01%     
==========================================
  Files         610      610              
  Lines       80387    80387              
==========================================
- Hits        33966    33964       -2     
- Misses      42240    42243       +3     
+ Partials     4181     4180       -1
Impacted Files Coverage Δ
services/pull/temp_repo.go 31.62% <0%> (-2.57%) ⬇️
models/unit.go 37.03% <0%> (-2.47%) ⬇️
services/pull/patch.go 62.89% <0%> (-1.89%) ⬇️
services/pull/check.go 56.64% <0%> (+2.09%) ⬆️
modules/process/manager.go 78.31% <0%> (+3.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbceca4...e8e3d4d. Read the comment docs.

@silverwind
Copy link
Member Author

Rebased and squashed.

@zeripath
Copy link
Contributor

@silverwind is there any way we can stop having to rebuild fomantic and the webpack every time we do make build?

My machine is FAST but it's taking a considerable amount of time to test one line changes because of this.

(We could also do with something similar for go generate - it's a real shame that bindata is regenerated despite no changes - I might take a look at that...)

@silverwind
Copy link
Member Author

silverwind commented Jan 26, 2020

@zeripath I thought 8ca0730 fixed those useless rebuilds. Try running make --trace build to see why it's building those particular targets.

@silverwind
Copy link
Member Author

Ah, I think I found something that triggers useless fomantic rebuilds, testing..

@silverwind
Copy link
Member Author

#9999 for possible workaround.

Regarding generate: Yes, I'd also prefer it to run on-demand but @guillep2k had some concerns about it last time I tried that, see #9114 (comment).

@zeripath
Copy link
Contributor

I was thinking of doing it in the generate functions themselves... They're the thing that is doing the work.

@silverwind
Copy link
Member Author

@zeripath possible, but I think make is ideal for timestamp checks like that.

Rebased here and also added a touch node_modules in case npm fails to update that for whatever reason.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 26, 2020
- added new 'make webpack' target
- deprecated 'make js' and 'make css'
- extend webpack config to load the less files
- updated docs

I had to rename the source file of `arc-green.less` to avoid generating
a useless JS entrypoint via webpack-fix-style-only-entries which would
not work with different source/destination filenames. I hear that there
should be cleaner solutions possible once we upgrade to Webpack 5.
@silverwind
Copy link
Member Author

Rebased

@zeripath zeripath merged commit 1019913 into go-gitea:master Jan 28, 2020
@silverwind silverwind deleted the webpack-css branch January 28, 2020 07:46
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants