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.

Sign up for GitHub

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

Jump to bottom

Update @wordpress/components package's contributing guidelines #33960

Merged
merged 35 commits into from
Sep 9, 2021
Merged

Update @wordpress/components package's contributing guidelines #33960

merged 35 commits into from
Sep 9, 2021

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Aug 9, 2021 •
edited
Loading

Description

Update the README and CONTRIBUTING files in the @wordpress/components package to reflect the latest guiding principles used by the package maintainers.

Next steps (to be carried out in separate PRs):

  • @diegohaz to work on:

    • Expand composition section
      • Composition patterns
      • (Semi-)Controlled components
      • Layout "responsibilities"
    • Expand API consistency section
    • Potentially add a "performance" section (@diegohaz)

  • @ciampo to work on:

    • integrate the CONTRIBUTING and README files under packages/components/src/ into the new guildlines
    • add a style guide on how to write documentation

How has this been tested?

Human reading!

Screenshots

N/A

Types of changes

New feature (updated / expanded docs)

Checklist:

  • N/A My code is tested.
  • N/A My code follows the WordPress code style.
  • N/A My code follows the accessibility standards.
  • N/A I've tested my changes with keyboard and screen readers.
  • N/A My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • N/A I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@ciampo ciampo marked this pull request as ready for review August 11, 2021 17:11
@ciampo
Copy link
Contributor Author

ciampo commented Aug 11, 2021

cc @griffbrad @sarayourfriend @diegohaz

I've started writing down a draft for an update CONTRIBUTING guidelines for the @wordpress/components package.

It'd be great if you could give it a read and help expand on a few sections that, for now, I've only written a few notes for.

sarayourfriend
sarayourfriend reviewed Aug 11, 2021
View reviewed changes
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
@ciampo ciampo added [Type] Developer Documentation Documentation for developers [Status] In Progress Tracking issues with work in progress [Feature] Component System WordPress component system [Package] Components /packages/components labels Aug 12, 2021
youknowriad
youknowriad reviewed Aug 17, 2021
View reviewed changes
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
gziolo
gziolo reviewed Aug 17, 2021
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

That's a great initiative. Thank you for starting looking at bringing some structure and better organization to @wordpress/components 👏🏻

packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
priethor
priethor reviewed Aug 17, 2021
View reviewed changes
packages/components/README.md Outdated Show resolved Hide resolved
packages/components/CONTRIBUTING.md Outdated Show resolved Hide resolved
This was referenced Aug 25, 2021
Closed
Open
Merged
@ciampo ciampo force-pushed the update/components-contributing-guidelines branch from e485ea6 to 0a36f58 Compare September 1, 2021 10:31
@gziolo
Copy link
Member

gziolo commented Sep 3, 2021

You might want to split this PR. It's getting big already 😄 Let us know when it's ready for the final review.

@ciampo
Copy link
Contributor Author

ciampo commented Sep 3, 2021

Let us know when it's ready for the final review.

It's not quite there yet — yesterday we agreed that @diegohaz is going to contribute to the "composition", "API consistency" and potentially a "performance" section (I've updated the PR description to keep track of these tasks)

You might want to split this PR. It's getting big already 😄

That makes sense! Let's wait for the bulk of the work to be done, and then we can make a decision on if/how to split this PR

@gziolo
Copy link
Member

gziolo commented Sep 3, 2021

That makes sense! Let's wait for the bulk of the work to be done, and then we can make a decision on if/how to split this PR

Actually, I was thinking more of merging sooner some parts that you consider ready and continue working on the rest in this PR 😄

ciampo and others added 17 commits September 8, 2021 17:12
@ciampo
Fix typo
d394f5d
@ciampo
Expand hooks vs components section, move under composition
3359502
@ciampo
Move sub components naming convention under "APIs consistency" section
cfc843d
@ciampo
Add Polymorphic components sub-section, reorganise "Components compos…
abf50ad 
…ition" section into sub-sections
@ciampo
Add more details about listing props through knobs in storybook
0021dfc
@ciampo
Remove hyphen from subcomponents
15f7023
@ciampo
Add content to the Context System section
4520f7e
@ciampo
comment out unfinied sections
95de8fb
@ciampo @diegohaz @gziolo
Apply suggestions from code review
5393b9a 
Co-authored-by: Haz <hazdiego@gmail.com>
Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
@ciampo
Comment out reference to the "composition section"
6647446
@ciampo
Rename "Emotion" section to "Styling"
670ca6c
@ciampo
Updale unit test section to link directly to components snapshot testing
e65518d
@ciampo
Update the storybook section, adding more details on stories and knobs
b6edc2e
@ciampo
Link to the Coding Guidelines in the "Documentation" section
e8bb18d
@ciampo
remove note about relevance of SCSS-related docs in README
08cd356
@ciampo
Update context section with example snippets
c34f731
@ciampo
Format internal links to absolute path format
8c494c8
@ciampo ciampo force-pushed the update/components-contributing-guidelines branch from a834aaf to 8c494c8 Compare September 8, 2021 17:25
@ciampo
Copy link
Contributor Author

ciampo commented Sep 8, 2021

Thank you @diegohaz and @gziolo for your last round of review! I've rebased the PR and addressed and/or replied to all of your points — I believe that we're going to be able to merge this PR very soon!

gziolo
gziolo approved these changes Sep 9, 2021
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

This is looking great. Let’s land and iterate as discussed above.

Excellent work documenting every detail ❤️

packages/components/README.md
@@ -10,7 +10,7 @@ Install the module
npm install @wordpress/components --save
```

_This package assumes that your code will run in an **ES2015+** environment. If you're using an environment that has limited or no support for ES2015+ such as IE browsers then using [core-js](https://github.com/zloirock/core-js) will add polyfills for these methods._
_This package assumes that your code will run in an **ES2015+** environment. If you're using an environment that has limited or no support for such language features and APIs, you should include [the polyfill shipped in `@wordpress/babel-preset-default`](/packages/babel-preset-default#polyfill) in your code._
Copy link
Member

Choose a reason for hiding this comment

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

Good job with this note’s update. I need to copy it to all other packages 😃

Copy link
Member

Choose a reason for hiding this comment

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

Opened #34878 to propagate the same change to other packages.

diegohaz
diegohaz approved these changes Sep 9, 2021
View reviewed changes
Copy link
Member

@diegohaz diegohaz left a comment

Choose a reason for hiding this comment

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

Awesome! Great work @ciampo!

@ciampo ciampo merged commit d3fa6c3 into trunk Sep 9, 2021
@ciampo ciampo deleted the update/components-contributing-guidelines branch September 9, 2021 09:08
@github-actions github-actions bot added this to the Gutenberg 11.6 milestone Sep 9, 2021
This was referenced Sep 9, 2021
Open
Closed
Open
@diegohaz diegohaz mentioned this pull request Sep 9, 2021
Merged
fullofcaffeine added a commit that referenced this pull request Sep 9, 2021
@fullofcaffeine
Merge branch 'trunk' into replace/confirm-et-al
5fd979f 
* trunk:
  [RNMobile][Embed block] Add device's locale to preview content (#33858)
  Update AlignmentMatrixControl docs post merge. (#34662)
  Chore: Update caniuse package to the latest version (#34685)
  Chore: Move `react-native-url-polyfill` to dev dependencies (#34687)
  Site Editor - add basic plugin support (#34460)
  ESLint Plugin: Use Jest related rules only when the package is installed (#33120)
  Update `@wordpress/components` package's contributing guidelines (#33960)
  chore(release): publish
  Update changelog files
  [RNMobile] [Embed block] Fix content disappearing on Android when switching light/dark mode (#34207)
  Scripts: Convert legacy entry point arguments for compatibility with webpack 5 (#34264)
  Update justication control in `flex` layout (#34651)
  Block Editor: Rename experimental prop used in `BlockControls` (#34644)
  Fix social links deprecation (#34639)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

@diegohaz diegohaz diegohaz approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@priethor priethor Awaiting requested review from priethor

@sarayourfriend sarayourfriend Awaiting requested review from sarayourfriend

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees
No one assigned
Labels
[Feature] Component System WordPress component system [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Developer Documentation Documentation for developers
Projects
None yet
Milestone
Gutenberg 11.6
Development

Successfully merging this pull request may close these issues.
6 participants
@ciampo @gziolo @youknowriad @diegohaz @sarayourfriend @priethor