Modernizr: Add simple test for the property value

Created on 2 Sep 2020 · 8Comments · Source: Modernizr/Modernizr

Hi 👋

Note: Check the comment below

What is proposed

This should be a relatively easy PR to get done. Every Modernizr test has a property tag on their metadata, this tag is always* lowercase without any punctuation. The idea would be to create a test that throws an error* whenever a property doesn't respect by this rule.

*The Modernizr website says that this rule is just a suggestion but in reality it's more a rule than a recommendation. Feel free to throw a warning instead of an error if you prefer.

How to do it

This test should be implemented in the lib\metadata.js file, a recent similar PR was approved (#2551) which added a similar test to this. After adding the test we would need a test for the test, this one would need to be implemented in test\node\lib\metadata.js file, you may check #2251 too (although the git diff is less readable in this one)

Feel free to raise any question in this issue if you stumble with problems :)

good first issue

Source

Markel

👍1

Most helpful comment

I love the idea of better issue templates, it would really help in a lot of areas (bug reportes, maintainers etc)

As for slack /discord/gitter: I like that there is only one place for working on this project. Everything is here in github and can be discussed in issues. Adding some other software/website into the mix would add more work keeping up to date with bug reports / questions while not offering more value.

Modernizr isnt the most relevant piece of software anymore so we should keep it simple :-)

rejas on 9 Sep 2020

👍2

All 8 comments

Okay, I should have checked before writing an issue 🤦. An error is actually thrown if you put uppercase symbols, but not for symbols like spaces or others. So yeah, the issue should continue, but you may want to check this issue to do the work. (It may actually be easier than before!)

Markel on 3 Sep 2020

Hi, can I work on this issue?

adzo261 on 6 Sep 2020

🚀1

Sure, go for it

rejas on 6 Sep 2020

👍2

Merged!

rejas on 8 Sep 2020

I don't know, but I quite like this format to create small changes that need to be done. Like explaining the things related to the change and give like some idea from where to start. What do you think @rejas ? @adzo261, maybe you could give us some feedback :)

Markel on 8 Sep 2020

I don't know, but I quite like this format to create small changes that need to be done. Like explaining the things related to the change and give like some idea from where to start. What do you think @rejas ? @adzo261, maybe you could give us some feedback :)

Yes, I like this way of presenting an issue. It was indeed helpful for me. Pointers on where to start for small issues and conceptual overview for striking changes can make it less overwhelming for new comers. Also, I would vote for having an issues' template if we don't have already.

And regarding the codebase, it is well structured and maintained. And members and contributors, you guys rock🤘.
I thoroughly enjoyed the interaction we had while working on it.
@Markel @rejas
I would love to stay connected, and may be try to work again in future.
Do we have slack or discord or gitter setup?

Thanks!

adzo261 on 8 Sep 2020

❤1

Also, I would vote for having an issues' template if we don't have already.

Yeah, I had been thinking of issuing a PR to add issue templates (including test requests, bugs, ...) for a while, but I just have been too lazy to start writing them 😅. I'll probably will start working on it next.