-
Notifications
You must be signed in to change notification settings - Fork 2.9k
chore: Updating Pull Request Template #25683
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
Conversation
## PR Description This PR updates the PR Template so that it reads `## Previous Behavior` instead of `## Current Behavior` when referring to the behavior that existed before the changes in the PR. This makes it less ambiguous as `Current` could mean the behavior before or after the PR depending on how it's used.
Asset size changesUnable to find bundle size details for Baseline commit: fc94f09 Possible causes
Recommendations
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f0b30dd:
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1148 | 1267 | 5000 | |
| Button | mount | 845 | 848 | 5000 | |
| FluentProvider | mount | 1540 | 1502 | 5000 | |
| FluentProviderWithTheme | mount | 576 | 564 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 531 | 527 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 555 | 629 | 10 | |
| MakeStyles | mount | 1768 | 1837 | 50000 | |
| SpinButton | mount | 2391 | 2305 | 5000 |
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| TreeWith60ListItems.default | 136 | 114 | 1.19:1 |
| AlertMinimalPerf.default | 220 | 190 | 1.16:1 |
| GridMinimalPerf.default | 276 | 240 | 1.15:1 |
| TextMinimalPerf.default | 280 | 246 | 1.14:1 |
| RefMinimalPerf.default | 178 | 159 | 1.12:1 |
| ImageMinimalPerf.default | 311 | 285 | 1.09:1 |
| TextAreaMinimalPerf.default | 383 | 356 | 1.08:1 |
| TableMinimalPerf.default | 333 | 313 | 1.06:1 |
| AvatarMinimalPerf.default | 158 | 151 | 1.05:1 |
| CheckboxMinimalPerf.default | 1684 | 1601 | 1.05:1 |
| FormMinimalPerf.default | 324 | 309 | 1.05:1 |
| AccordionMinimalPerf.default | 120 | 115 | 1.04:1 |
| FlexMinimalPerf.default | 235 | 227 | 1.04:1 |
| LabelMinimalPerf.default | 321 | 308 | 1.04:1 |
| ListWith60ListItems.default | 517 | 495 | 1.04:1 |
| LoaderMinimalPerf.default | 276 | 265 | 1.04:1 |
| SkeletonMinimalPerf.default | 291 | 279 | 1.04:1 |
| StatusMinimalPerf.default | 569 | 549 | 1.04:1 |
| AttachmentMinimalPerf.default | 121 | 118 | 1.03:1 |
| AttachmentSlotsPerf.default | 944 | 913 | 1.03:1 |
| ChatMinimalPerf.default | 606 | 589 | 1.03:1 |
| ButtonMinimalPerf.default | 136 | 133 | 1.02:1 |
| CarouselMinimalPerf.default | 382 | 373 | 1.02:1 |
| ListMinimalPerf.default | 424 | 414 | 1.02:1 |
| PopupMinimalPerf.default | 530 | 519 | 1.02:1 |
| ProviderMinimalPerf.default | 337 | 330 | 1.02:1 |
| SegmentMinimalPerf.default | 291 | 284 | 1.02:1 |
| ToolbarMinimalPerf.default | 772 | 758 | 1.02:1 |
| TooltipMinimalPerf.default | 1991 | 1943 | 1.02:1 |
| AnimationMinimalPerf.default | 437 | 432 | 1.01:1 |
| BoxMinimalPerf.default | 277 | 274 | 1.01:1 |
| DatepickerMinimalPerf.default | 4768 | 4739 | 1.01:1 |
| DropdownMinimalPerf.default | 2255 | 2236 | 1.01:1 |
| ItemLayoutMinimalPerf.default | 971 | 962 | 1.01:1 |
| MenuButtonMinimalPerf.default | 1414 | 1400 | 1.01:1 |
| RosterPerf.default | 1761 | 1735 | 1.01:1 |
| ProviderMergeThemesPerf.default | 1066 | 1054 | 1.01:1 |
| ReactionMinimalPerf.default | 308 | 306 | 1.01:1 |
| SliderMinimalPerf.default | 1326 | 1312 | 1.01:1 |
| HeaderMinimalPerf.default | 291 | 290 | 1:1 |
| HeaderSlotsPerf.default | 619 | 618 | 1:1 |
| LayoutMinimalPerf.default | 288 | 287 | 1:1 |
| ListCommonPerf.default | 512 | 513 | 1:1 |
| ListNestedPerf.default | 444 | 446 | 1:1 |
| IconMinimalPerf.default | 542 | 541 | 1:1 |
| TreeMinimalPerf.default | 664 | 666 | 1:1 |
| ChatDuplicateMessagesPerf.default | 213 | 215 | 0.99:1 |
| ChatWithPopoverPerf.default | 296 | 298 | 0.99:1 |
| DividerMinimalPerf.default | 292 | 294 | 0.99:1 |
| PortalMinimalPerf.default | 135 | 136 | 0.99:1 |
| CustomToolbarPrototype.default | 2207 | 2223 | 0.99:1 |
| DropdownManyItemsPerf.default | 529 | 541 | 0.98:1 |
| RadioGroupMinimalPerf.default | 358 | 364 | 0.98:1 |
| SplitButtonMinimalPerf.default | 3536 | 3607 | 0.98:1 |
| CardMinimalPerf.default | 409 | 423 | 0.97:1 |
| MenuMinimalPerf.default | 670 | 688 | 0.97:1 |
| VideoMinimalPerf.default | 566 | 583 | 0.97:1 |
| ButtonOverridesMissPerf.default | 1030 | 1078 | 0.96:1 |
| EmbedMinimalPerf.default | 2867 | 2990 | 0.96:1 |
| InputMinimalPerf.default | 889 | 934 | 0.95:1 |
| DialogMinimalPerf.default | 587 | 631 | 0.93:1 |
| TableManyItemsPerf.default | 1399 | 1530 | 0.91:1 |
| ButtonSlotsPerf.default | 411 | 456 | 0.9:1 |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1074 | 1828 | 5000 | Possible regression |
| Label | mount | 733 | 1299 | 5000 | Possible regression |
| Layer | mount | 5960 | 4650 | 5000 | Possible regression |
| Text | mount | 1151 | 790 | 5000 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1074 | 1828 | 5000 | Possible regression |
| Breadcrumb | mount | 3056 | 2863 | 1000 | |
| Checkbox | mount | 2600 | 2526 | 5000 | |
| CheckboxBase | mount | 2177 | 2322 | 5000 | |
| ChoiceGroup | mount | 4288 | 5867 | 5000 | |
| ComboBox | mount | 1232 | 1224 | 1000 | |
| CommandBar | mount | 11525 | 9934 | 1000 | |
| ContextualMenu | mount | 13302 | 13166 | 1000 | |
| DefaultButton | mount | 1374 | 1374 | 5000 | |
| DetailsRow | mount | 3500 | 3602 | 5000 | |
| DetailsRowFast | mount | 3614 | 3436 | 5000 | |
| DetailsRowNoStyles | mount | 3480 | 3538 | 5000 | |
| Dialog | mount | 3175 | 3169 | 1000 | |
| DocumentCardTitle | mount | 581 | 600 | 1000 | |
| Dropdown | mount | 3246 | 3262 | 5000 | |
| FocusTrapZone | mount | 2043 | 1998 | 5000 | |
| FocusZone | mount | 1979 | 2006 | 5000 | |
| GroupedList | mount | 1969 | 2236 | 2 | |
| GroupedList | virtual-rerender | 1137 | 1057 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 1726 | 1553 | 2 | |
| GroupedListV2 | mount | 571 | 588 | 2 | |
| GroupedListV2 | virtual-rerender | 536 | 569 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 593 | 584 | 2 | |
| IconButton | mount | 1903 | 1844 | 5000 | |
| Label | mount | 733 | 1299 | 5000 | Possible regression |
| Layer | mount | 5960 | 4650 | 5000 | Possible regression |
| Link | mount | 768 | 785 | 5000 | |
| MenuButton | mount | 1709 | 1658 | 5000 | |
| MessageBar | mount | 2349 | 2366 | 5000 | |
| Nav | mount | 3277 | 3326 | 1000 | |
| OverflowSet | mount | 1377 | 1334 | 5000 | |
| Panel | mount | 2608 | 2644 | 1000 | |
| Persona | mount | 1288 | 1319 | 1000 | |
| Pivot | mount | 1639 | 1667 | 1000 | |
| PrimaryButton | mount | 1537 | 1538 | 5000 | |
| Rating | mount | 7002 | 7130 | 5000 | |
| SearchBox | mount | 1534 | 1527 | 5000 | |
| Shimmer | mount | 2875 | 2935 | 5000 | |
| Slider | mount | 2090 | 2140 | 5000 | |
| SpinButton | mount | 8758 | 4714 | 5000 | |
| Spinner | mount | 798 | 786 | 5000 | |
| SplitButton | mount | 2922 | 3122 | 5000 | |
| Stack | mount | 846 | 830 | 5000 | |
| StackWithIntrinsicChildren | mount | 2437 | 2475 | 5000 | |
| StackWithTextChildren | mount | 4927 | 4791 | 5000 | |
| SwatchColorPicker | mount | 10068 | 10584 | 5000 | |
| TagPicker | mount | 2550 | 2642 | 5000 | |
| TeachingBubble | mount | 104197 | 106106 | 5000 | |
| Text | mount | 1151 | 790 | 5000 | Possible regression |
| TextField | mount | 1620 | 1574 | 5000 | |
| ThemeProvider | mount | 1449 | 1545 | 5000 | |
| ThemeProvider | virtual-rerender | 1088 | 1096 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2140 | 2126 | 5000 | |
| Toggle | mount | 1120 | 1138 | 5000 | |
| buttonNative | mount | 538 | 520 | 5000 |
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.
Looks good to me.
And this is unrelated, but another minor change while you're here... would you mind adding a * in front of Fixes on line 27? That will make it into a bullet, but more helpfully, it'll cause GitHub to format it with the issue's full title instead of just the number.
* Fixes #
I can also make a separate PR if you'd prefer.
@behowell done! |
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
🕵 fluentuiv8 No visual regressions between this PR and main |
🕵 fluentuiv9 Open the Visual Regressions report to inspect the 1 screenshots✅ There was 0 screenshots added, 0 screenshots removed, 1750 screenshots unchanged, 0 screenshots with different dimensions and 1 screenshots with visible difference. unknown 1 screenshots
|
|
next time it would be nice to get all teams consensus on changes that affect all teams and all contributors. Would you mind elaborate the pressing concern :) ? Also regarding the bullet with Fixes seems odd. It also might break closing referenced issues 🥲. Last but not least this template was based on common OSS practices. |
|
@Hotell, I remember we had this conversation before in Teams a while ago and it was agreed to change from |
* chore: Updating Pull Request Template ## PR Description This PR updates the PR Template so that it reads `## Previous Behavior` instead of `## Current Behavior` when referring to the behavior that existed before the changes in the PR. This makes it less ambiguous as `Current` could mean the behavior before or after the PR depending on how it's used. * Update PULL_REQUEST_TEMPLATE.md
Since this was my suggestion, I can confirm that it does not break closing referenced issues. I've been adding bullets to my PRs without any problems (e.g. #25497). Can you elaborate on your comment that it seems odd? See my comment above for the reasoning behind it: #25683 (review) |
👍
Not sure if this needs more elaboration ? -> "Also regarding the bullet with Fixes seems odd. It also might break closing referenced issues 🥲." |
* chore: Updating Pull Request Template ## PR Description This PR updates the PR Template so that it reads `## Previous Behavior` instead of `## Current Behavior` when referring to the behavior that existed before the changes in the PR. This makes it less ambiguous as `Current` could mean the behavior before or after the PR depending on how it's used. * Update PULL_REQUEST_TEMPLATE.md
PR Description
This PR updates the PR Template so that it reads
## Previous Behaviorinstead of## Current Behaviorwhen referring to the behavior that existed before the changes in the PR. This makes it less ambiguous asCurrentcould mean the behavior before or after the PR depending on how it's used.