Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
v5.9.0 #4491
v5.9.0 #4491
Changes from all commits
e0ebae4
825259a
bc037ab
0ed67f0
563ecc3
9cc765c
669c595
a30b2e8
b47ccc7
47fffdc
32b3204
b970bce
ff5dea1
c5b1c14
30b889d
2a7dd7a
c2c0fd0
485b3f7
36e8fae
17261b4
b2b015b
13c1fb1
8e7848a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It is currently not very clear which packages are impacted by these core changes.
Sometime it is a change cross package (for the doc gen for instance)
But sometimes it is a change inside one of the package.
Should we prefix by the component family here instead of "Core" now that we have several ?
For instance
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.
Can we keep
[core]
prefix and use label to differentiate between data grid and pickers?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 do not see a big interest in replacing
[core]
because developers do not really need to read those lines.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.
It is mostly for us, as the team grow I think it should be clear looking at a PR if it impacts the pickers or the datagrid, or the charts, etc...
But the label could work.
That would mean put the "data grid" label on the "[core]" PR that modifies code on the grid packages (same for pickers).
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.
Makes total sense to me.
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 core repo seems to use
[core]
only for changes in the infra. I think we used[core]
in the past because there was a lot of changes in internal parts. Maybe now, with two components, makes sense to use the prefix of the component affected. The "Add technical doc for pipe processing and family processing" item I would keep with[core]
because I understand that[docs]
is only for changes that affect the deployed docs.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.
Agree
As for moving everything not infra inside the package sections, I don't have a strong preference