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.

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

PublicApiAnalyzer refinements #3677

Merged
merged 41 commits into from
May 27, 2021
Merged

Conversation

tobias-tengler
Copy link
Collaborator

@tobias-tengler tobias-tengler commented May 7, 2021

  • Enforce documentation of public API only for PRs targeting the main branch
  • Separate pipeline job/Nuke target for checking the public API
  • Convert powershell scripts to Nuke
  • Exclude PublicAPI files from search results
  • Add new APIs to unshipped

@tobias-tengler

This comment has been minimized.

@michaelstaib
Copy link
Member

I will have a look.

@tobias-tengler tobias-tengler force-pushed the tte/public-api-analyzer-refinements branch from d90fb32 to 5ecdf59 Compare May 10, 2021 12:29
@michaelstaib
Copy link
Member

I will do this tomorrow. :)

@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented May 13, 2021

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

This Code Smell should be ignored. It's that the parameter name on the implementation is different from the one on the interface. I removed this Code Smell in another PR, but technically it's a breaking change. Therefor I reverted it and essentially just undid my previous change:

https://github.com/ChilliCream/hotchocolate/pull/3677/files#diff-da4e9423d08f59f3dd6a29541a5b52914417755475add15c36627fcecaa80a9dL116-R117

@tobias-tengler tobias-tengler force-pushed the tte/public-api-analyzer-refinements branch from 8590f48 to 53c64fd Compare May 24, 2021 08:32
@tobias-tengler tobias-tengler force-pushed the tte/public-api-analyzer-refinements branch from 53c64fd to c756c42 Compare May 24, 2021 08:33
@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented May 25, 2021

@michaelstaib I added the remaining Nuke targets and also tested the GitHub action in my fork of HotChocolate.
This PR should be good to go! 🚀

EDIT: Okay it's not good to go - the Nuke update apparently broke the build ^^

@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented May 25, 2021

@michaelstaib I'm not exactly sure how the changes here have messed with the build/tests, but it seems like we should install all the frameworks we're targeting in the build pipeline, e.g. net5.0 and netcoreapp3.1.
Should I do this or do you know something else that could be the cause?

@michaelstaib
Copy link
Member

This Code Smell should be ignored. It's that the parameter name on the implementation is different from the one on the interface. I removed this Code Smell in another PR, but technically it's a breaking change. Therefor I reverted it and essentially just undid my previous change:

we can ignore this since the API is obsolete and will be removed in future. I will override the validation.

@michaelstaib
Copy link
Member

Build seems to work now again. I updated the devops pipelines to also install the old 3.1 framework.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@michaelstaib
Copy link
Member

The red tests are flaky ... will deal with them in a separate issue.

@michaelstaib michaelstaib merged commit e5c0595 into develop May 27, 2021
@michaelstaib michaelstaib deleted the tte/public-api-analyzer-refinements branch May 27, 2021 22:19
tobias-tengler added a commit that referenced this pull request Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants