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

Fill the documentation #2561

Merged
merged 28 commits into from Jul 2, 2020
Merged

Fill the documentation #2561

merged 28 commits into from Jul 2, 2020

Conversation

Markel
Copy link
Contributor

@Markel Markel commented May 13, 2020

Adds these new sections to the HOW_TO_WRITE_FEATURE_DETECTS.md:

  • Polyfills
  • Submitting a PR (simple)
  • Testing
  • Install the basics (simplistic)
  • Multiple rephrases for better readability

It also reorders some files to follow alphabetical order.

@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #2561 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2561   +/-   ##
=======================================
  Coverage   95.15%   95.15%           
=======================================
  Files           5        5           
  Lines         165      165           
=======================================
  Hits          157      157           
  Misses          8        8           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd28403...6f838e7. Read the comment docs.

@Markel
Copy link
Contributor Author

Markel commented May 28, 2020

I'll soon solve the conflicts

@Markel
Copy link
Contributor Author

Markel commented May 28, 2020

Okay merge conflicts solved, still there is a broken link link with the Verify/Run your tests (seems that Github Markdown parser doesn't like slashes but the url redirect yes, etc) so I'll probably will change the name in the next commit (I assume that some extra tweaking will be needed).
Also, the install the basics section doesn't end up convincing me :/

@Markel Markel requested a review from rejas June 3, 2020 11:09
Copy link
Member

@rejas rejas left a comment

Choose a reason for hiding this comment

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

lgtm. what do you think @patrickkettner ?

@rejas rejas requested a review from patrickkettner June 5, 2020 08:28
@Markel
Copy link
Contributor Author

Markel commented Jun 9, 2020

🤔 I was thinking about the testing section, maybe it would be a good idea to put the General testing & Caniuse testing at level 4 under a Create Tests section (level 3) so it is more obvious that those 2 are for creating and different from the verify and running section.

Moreover, the testing section would be duplicated if we merge this PR (check the README)

@rejas
Copy link
Member

rejas commented Jul 2, 2020

Since @patrickkettner went dark again, I'd merge it even without his review. Or what do you say @MarkelFe ?

@Markel
Copy link
Contributor Author

Markel commented Jul 2, 2020

Up to you @rejas

I mean, I think that nothing is technically wrong, so suggestions and improvements (some even mentioned by me) can be added later down the road. But yeah, up to you 🙃

@rejas rejas merged commit f81995d into Modernizr:master Jul 2, 2020
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.

None yet

2 participants