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

Document separate "build" from "minify/uglify" tasks in grunt #875

Closed
jywarren opened this issue Mar 17, 2019 · 2 comments
Closed

Document separate "build" from "minify/uglify" tasks in grunt #875

jywarren opened this issue Mar 17, 2019 · 2 comments

Comments

@jywarren
Copy link
Member

@publiclab/is-reviewers this was suggested already but we didn't follow-up, so I'm opening a new issue for it. It seems this exists now, but it's not well documented.

Copying in the last note from that issue via @harshkhandeparkar on this distinct issue:

Yes. Definitely. I think the gruntfile should be revised. Its very messy as of now. The ui dist files are not built in the build task. All the targets of a task are mentioned whereas the whole taskname can be used. Like instead of adding browserify:dist browserify:js uglify:dist uglify:js to the build task, browserify ulgify is enough. Etc. Can somebody open a new issue for that or do taht directly?

So, I think we currently have:

  1. grunt build - just do everything below
  2. grunt browserify - just generate dist/image-sequencer.js
  3. grunt uglify - minify dist/image-sequencer.jstodist/image-sequencer.min.js`

Is this correct? I believe we could also better show building UI files, so instead of current grunt browserify:js it should be grunt browserify:ui?

https://github.com/publiclab/image-sequencer/blob/main/Gruntfile.js#L56

Let's especially make it clear to people that:

  1. you don't need to submit /dist/ files (as per Managing dist files #601) in PRs
  2. while developing you don't need to run full grunt build but only grunt browserify?

Is this correct?

@jywarren
Copy link
Member Author

Reopening just so we can add better documentation to newcomers using this! (and oldcomers since it changed 😄 )

@jywarren jywarren reopened this Mar 18, 2019
@harshkhandeparkar
Copy link
Member

Fixed in #897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants