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

Updated PostCSS, and added warning when variable cannot be resolved along with option to turn warning on or off ('warnOfUnresolved'). Upped version. Etc. #37

Merged
merged 7 commits into from
Dec 29, 2017

Conversation

mjschlot
Copy link
Contributor

@mjschlot mjschlot commented Nov 26, 2017

  • Updated postcss dependency to 6
  • Made required updates due to upgrade to version 6
  • Added the new postcss 'results' parameter so I can add warnings (via results.warn(..))
  • Added option 'warnOfUnresolved' and defaulted to true.
  • Documented new option.
  • Updated the version to 2.0.0. since the update of postcss dependency
  • Updated eslint, and postcss-scss to newest versions
  • Added "postcss-reporter" as dev dependency so we can see postcss warnings when runing tests. Helpful when resolving some bugs uncovered when I started reporting warnings that were not obvious before, and also helpful when resolving issues resulting from upgrading postcss
  • Added and documented support for variables being a function instead of an object

@mjschlot
Copy link
Contributor Author

Hi @jonathantneal
Do you believe you will have time to merge this change before end of year? I'm planning some changes to my team's use of PostCSS and knowing timing would be helpful.
Thanks!

@deemstone
Copy link

Seems a little complex. Transparent result through the functions calls. Make this PR looks not relieved.

@jonathantneal
Copy link
Collaborator

There is a lot going on in this PR. My thoughts are:

  1. we need more maintainers for this project.
  2. we need a maintainer for precss.

Actually, I stopped maintaining precss almost immediately after it was released, going as far as to not follow the project. I am just now seeing how many issues it has and how popular it remains despite that — which makes no sense to me.

This is a lot to take in.

@mjschlot
Copy link
Contributor Author

@deemstone Yes, passing result around is not my preferred solution either, but other solutions would require a more significant rewrite, and this is what works with the current coding -- and the current coding is good and I see no reason to do a larger rewrite.

@jonathantneal I agree. It is a lot. There are a few extras but 95% of it was mandatory to accomplish adding warnings via postcss 6's new API, or were directly the results of upgrading to postcss 6. The other 5% are things myself and my team really wanted to see (e.g. warnings on unresolved variables). With this larger commit merged in future PRs can be much smaller to add some new features and fixes. But this larger PR seems unavoidable to get postcss-advanced-variables upgrade to postcss 6.

How can I help get this merged and help maintain postcss-advanced-variables? I don't have the time to help with precss, but I can help with this postcss-advanced-variables if you want.

@jonathantneal jonathantneal merged commit 2c45523 into csstools:master Dec 29, 2017
@mjschlot
Copy link
Contributor Author

Thanks @jonathantneal . Now that it is merged in, can it be published to NPM?

jonathantneal pushed a commit that referenced this pull request Jan 1, 2018
…long with option to turn warning on or off ('warnOfUnresolved'). Upped version. Etc. (#37)

* v2.0.0

* Updated PostCSS, and added warning when variable can not be resolved along with option to turn warning on or off ('warnOfUnresolved'). Upped version.

* Reverted most eslintrc settings back.

* Added yarn.lock

* Work on adding function option for variables option. Added unit test, and updated documentation. This is my first pass.

* Reordered arguments of getVariableTransformedString, and added the new css test file for testing the variables as function option.

* Updated test to test the second argument of the variables function option.
jonathantneal pushed a commit that referenced this pull request Jan 1, 2018
…long with option to turn warning on or off ('warnOfUnresolved'). Upped version. Etc. (#37)

* v2.0.0

* Updated PostCSS, and added warning when variable can not be resolved along with option to turn warning on or off ('warnOfUnresolved'). Upped version.

* Reverted most eslintrc settings back.

* Added yarn.lock

* Work on adding function option for variables option. Added unit test, and updated documentation. This is my first pass.

* Reordered arguments of getVariableTransformedString, and added the new css test file for testing the variables as function option.

* Updated test to test the second argument of the variables function option.
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.

3 participants