-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
v4.12.3 #27465
v4.12.3 #27465
Conversation
Details of bundle changes.Comparing: e02afb6...bb19dfa Details of page changes
|
Co-authored-by: Marija Najdova <[email protected]>
### `@material-ui/[email protected]` | ||
|
||
- <!-- 2 --> [AccordionSummary] Fix false-positive propType warning with `disableGeneration` (#27385) @eps1lon | ||
- <!-- 6 --> [TablePagination] Re-introduce deprecated `onChangePage` to `ActionsComponent` (#27407) @eps1lon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component is the Table?
- <!-- 6 --> [TablePagination] Re-introduce deprecated `onChangePage` to `ActionsComponent` (#27407) @eps1lon | |
- <!-- 6 --> [Table] Re-introduce deprecated TablePagination `onChangePage` to `ActionsComponent` (#27407) @eps1lon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We changed TablePagination
, not Table
:
https://github.com/mui-org/material-ui/pull/27407/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had tried to document the standard used so far in https://www.notion.so/material-ui/GitHub-issues-PRs-c1d7072e0c2545b0beb43b115f6030f6
A pull request most often has one primary label. It's defined with the "[XXX] Syntax". XXX must match with existing GitHub labels.
It's arbitrary, I think that what matters is to be consistent. "Table" matches an existing GitHub label.
If we want to change the standard, then it's a different discussion, why not.
I think that we should get it changed at the whole org level, not on an individual basis. It would help external developers read the changelog, the issues, the PRs, contribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't discuss it to begin with so I don't accept that standard. Your approach is not suitable for a changelog which is what we're discussing here.
You already agreed with my approach: The title should match the component. Though you mistook the PR to adress the Table
component. However, it did adress the TablePagination
component so that's the correct title.
I think that we should get it changed at the whole org level, not on an individual basis.
Agreed. There's no org-level at the moment though.
|
||
### `@material-ui/[email protected]` | ||
|
||
- <!-- 2 --> [AccordionSummary] Fix false-positive propType warning with `disableGeneration` (#27385) @eps1lon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component is the Accordion?
- <!-- 2 --> [AccordionSummary] Fix false-positive propType warning with `disableGeneration` (#27385) @eps1lon | |
- <!-- 2 --> [Accordion] Fix AccordionSummary false-positive propType warning with `disableGeneration` (#27385) @eps1lon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
Co-authored-by: Olivier Tassinari <[email protected]>
forgot in previous merge commit
No description provided.