Skip to content

[jenkins] Improve api/generator diff reporting to say if there were changes, and if they were breaking or not. - #4138

Merged
rolfbjarne merged 11 commits into
dotnet:masterfrom
rolfbjarne:jenkins-descriptive-diffs
May 30, 2018
Merged

[jenkins] Improve api/generator diff reporting to say if there were changes, and if they were breaking or not.#4138
rolfbjarne merged 11 commits into
dotnet:masterfrom
rolfbjarne:jenkins-descriptive-diffs

Conversation

@rolfbjarne

@rolfbjarne rolfbjarne commented May 28, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

For the api diff for this PR we now show:

  • 🔥 breaking changes 🔥: If there are any breaking changes in the api diff (for this PR).
  • please review changes: If there are any non-breaking changes in the api diff.
  • no change: If the api diff is empty.

For the generator diff we show:

  • only version changes: If there were only changes related to version numbers (since the XI/XM version number is added to the generator, that version number will always show up as a diff when comparing the generated source code)
  • please review changes: If anything other that version numbers changed in the generated source code.

@rolfbjarne rolfbjarne added skip-all-tests Skip all the tests do-not-merge Do not merge this pull request labels May 28, 2018
@rolfbjarne

rolfbjarne commented May 28, 2018

Copy link
Copy Markdown
Member Author

do-not-merge since I'll have to push a few test commits to actually test this properly.

@monojenkins

monojenkins commented May 29, 2018

Copy link
Copy Markdown
Contributor

Build success
Build comment file:

✅ Provisioning succeeded
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (🔥 breaking changes 🔥)
✅ Generator Diff (please review changes)
✅ Test run succeeded

rolfbjarne added 4 commits May 30, 2018 07:37
@rolfbjarne
Revert "Introduce generator change."
a9d6915 
This reverts commit 1ebbaa3.
@rolfbjarne
Revert "INTRODUCE BREAKING CHANGE"
680b438 
This reverts commit 074ed4b.
@rolfbjarne
Add non-breaking API.
c32acf5
@rolfbjarne
[compare] More advanced script block removal.
dd36808 
grep isn't capable enough to remove multiple script block in a file (at least
mac-api-diff.html contains multiple other files, each with its own script
block), so step up a bit and use perl instead.
@monojenkins

monojenkins commented May 30, 2018

Copy link
Copy Markdown
Contributor

Build success
Build comment file:

✅ Provisioning succeeded
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (please review changes)
✅ Generator Diff (only version changes)
✅ Test run succeeded

@dotnet dotnet deleted a comment from monojenkins May 30, 2018
@dotnet dotnet deleted a comment from monojenkins May 30, 2018
@dotnet dotnet deleted a comment from monojenkins May 30, 2018
@dotnet dotnet deleted a comment from monojenkins May 30, 2018
@rolfbjarne

rolfbjarne commented May 30, 2018

Copy link
Copy Markdown
Member Author

@spouliot this is ready for review now.

The two monojenkins comments above show how it shows up with breaking API changes and generator changes (first comment), and non-breaking API changes (second comment)

@monojenkins

monojenkins commented May 30, 2018

Copy link
Copy Markdown
Contributor

Build success
Build comment file:

✅ Provisioning succeeded
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (only version changes)
✅ Test run succeeded

VincentDondain
VincentDondain approved these changes May 30, 2018
View reviewed changes

@VincentDondain VincentDondain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool

@rolfbjarne
rolfbjarne merged commit e24a6f6 into dotnet:master May 30, 2018
@rolfbjarne rolfbjarne removed the do-not-merge Do not merge this pull request label May 30, 2018
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 12, 2018
@rolfbjarne
[jenkins] Improve api/generator diff reporting to say if there were c…
fa7f52d 
…hanges, and if they were breaking or not. (dotnet#4138)

For the api diff for this PR we now show:

* `🔥 breaking changes 🔥`: If there are any breaking changes in the api diff (for this PR).
* `please review changes`: If there are any non-breaking changes in the api diff.
* `no change`: If the api diff is empty.

For the generator diff we show:

* `only version changes`: If there were only changes related to version numbers (since the XI/XM version number is added to the generator, that version number will always show up as a diff when comparing the generated source code)
* `please review changes`: If anything other that version numbers changed in the generated source code.
@rolfbjarne rolfbjarne mentioned this pull request Jun 12, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dalexsoto dalexsoto dalexsoto approved these changes
+2 more reviewers
@spouliot spouliot spouliot approved these changes
@VincentDondain VincentDondain VincentDondain approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip-all-tests Skip all the tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rolfbjarne @monojenkins @dalexsoto @spouliot @VincentDondain