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

Knockout.js support and improved obfuscation. #31

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Knockout.js support and improved obfuscation. #31

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 9, 2018

No description provided.

@@ -1,19 +1,27 @@
{
"name": "gulp-selectors",
"description": "Minify CSS selectors.",
"version": "0.1.8",
"version": "0.1.10",
Copy link
Owner

Choose a reason for hiding this comment

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

We may want to bump the minor or major here instead of the patch, since this PR at least adds significant new functionality/support for JS. At most, the ownership change may be considered a breaking change to some.

@cazzer
Copy link
Owner

cazzer commented Dec 10, 2018

Thanks for this PR and offer to take ownership, @davismiculis. I'll spend some further time reviewing as soon as possible. Some considerations:

  1. I noticed you dropped a lot of tests; just a point of concern for any package that is part of a build process, (especially since the test were mostly around the core functionality)
  2. You are making a lot of Knockout-specific changes with this PR which looks like a great enhancement, do you still plan to support vanilla CSS and HTML going forward?
  3. Should we transfer ownership before merging? I can also give you ownership of the NPM package at the same time

@ghost
Copy link
Author

ghost commented Dec 11, 2018

Hi, I just created this MR to see changes. I no longer have my original fork. I currently have no plans to support this further. 🙄

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.

1 participant