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

Don't reminify *.min.js #6986

Merged
merged 1 commit into from May 6, 2016
Merged

Don't reminify *.min.js #6986

merged 1 commit into from May 6, 2016

Conversation

@laosb
Copy link
Collaborator

@laosb laosb commented May 6, 2016

#5378 Old PR
#5363 Issue

Resubmitted but I'm not ready with writing tests. Personally think that shouldn't have any problems. Pinging the member dealing with the old PR @stubailo

@laosb
Copy link
Collaborator Author

@laosb laosb commented May 6, 2016

Personally think .min.css shouldn't be reminified too.

@benjamn benjamn merged commit e955328 into meteor:devel May 6, 2016
1 of 2 checks passed
1 of 2 checks passed
ci/circleci Your tests failed on CircleCI
Details
CLA Author has signed the Meteor CLA.
Details
@benjamn
Copy link
Member

@benjamn benjamn commented May 6, 2016

This looks safe and beneficial to me.

@benjamn
Copy link
Member

@benjamn benjamn commented May 6, 2016

Feel free to submit another PR for .min.css :)

@laosb
Copy link
Collaborator Author

@laosb laosb commented May 7, 2016

Should this be mentioned in History.md and docs? There might be some cases that people use .min.js for other reasons but still want be minified.

@laosb laosb deleted the laosb:patch-13 branch May 7, 2016
@stubailo
Copy link
Contributor

@stubailo stubailo commented May 7, 2016

Definitely should be in the docs regardless!

@mitar
Copy link
Collaborator

@mitar mitar commented May 7, 2016

Yes, please add tests and docs as well, no?

@laosb
Copy link
Collaborator Author

@laosb laosb commented May 7, 2016

@mitar Learning how to write tests! Is there any document about it? When comes to docs, should I simply add it to the old Meteor-based docs or to the hexo-based docs in the PR?

@mitar
Copy link
Collaborator

@mitar mitar commented May 7, 2016

Just check how other tests are done and add to them. But I see that this package does not have them. So maybe then it is on MDG to write first ones. I do not know how to write tests for tooling.

@mitar
Copy link
Collaborator

@mitar mitar commented May 7, 2016

For documentation, yes, that is in flux currently.

@tmeasday
Copy link
Contributor

@tmeasday tmeasday commented May 7, 2016

I do not know how to write tests for tooling.

You can use selftest for this. I don't think it's well documented. We should fix that!

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.