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

CI automation to help upgrading docusaurus-theme-openapi #9287

Closed
chris48s opened this issue Jun 18, 2023 · 9 comments
Closed

CI automation to help upgrading docusaurus-theme-openapi #9287

chris48s opened this issue Jun 18, 2023 · 9 comments
Assignees
Labels
developer-experience Dev tooling, test framework, and CI frontend The React app and the infrastructure that supports it

Comments

@chris48s
Copy link
Member

📋 Description

There are two React components from docusaurus-theme-openapi which we have swizzled to customise them for our needs.

This means that when we upgrade the docusaurus-theme-openapi, if either of the components we have customised (Curl and Response) have changed in the upstream package, we need to review our custom components to see if any changes or updates are needed.

I want to write a GitHub action that can flag dependabot PRs bumping docusaurus-theme-openapi that require manual intervention to review or update these components. Something like close-bot is probably a reasonable starting point for what this should look like.

@chris48s chris48s added frontend The React app and the infrastructure that supports it developer-experience Dev tooling, test framework, and CI labels Jun 18, 2023
@calebcartwright
Copy link
Member

I want to write a GitHub action that can flag dependabot PRs bumping docusaurus-theme-openapi that require manual intervention to review or update these components. Something like close-bot is probably a reasonable starting point for what this should look like.

Is this proposed (I presume first step) flagging in the sense of essentially just posting some "hey Shields maintainer, pay close attention to this one" comment on the dependabot PR?

@chris48s
Copy link
Member Author

I think the easiest thing (so probably the first step) is to just do something like (as you say)

if packageName == 'docusaurus-theme-openapi' then post a comment on the PR saying "hey Shields maintainer, pay close attention to this one"

But I think we could look at more elaborate stuff. One option would be to post a diff of the files from the package against the ones from our repo, or diff the specific files in the old/new package versions and say if they have changed or not.

@jNullj
Copy link
Collaborator

jNullj commented Jul 28, 2023

Could give a hand with this implanting, assign me if you want.

@chris48s
Copy link
Member Author

If you wanted to pick this one up, that would be awesome. I think for a first implementation, just adding a warning to the PR saying:

  • This package contains components we've overridden
  • We need to watch out for changes to the Curl and Response components

is a great start.

https://github.com/badges/shields/tree/master/.github/actions/close-bot is probably a good point of reference for a similar-ish action we have written.

jNullj added a commit to jNullj/shields-fun-fork that referenced this issue Aug 9, 2023
Add new action and workflow that runs on new dependabot PR.
The action checks changes in components of docusaurus-theme-openapi  that are swizzled and warns maintainers to review those changes.
For more info see badges#9287

Solves badges#9287
jNullj added a commit to jNullj/shields-fun-fork that referenced this issue Aug 10, 2023
Add new action and workflow that runs on new dependabot PR.
The action checks changes in components of docusaurus-theme-openapi  that are swizzled and warns maintainers to review those changes.
For more info see badges#9287

Solves badges#9287
@jNullj
Copy link
Collaborator

jNullj commented Aug 10, 2023

So here is what i came up with (The old and new are mixed due to the way i am testing, it should not be an issue)
image
It will inform which components are changed, the list on the last row is dynamic, the script tests what files are changed in the diff of the tags for the two versions.

The only issue i am still testing is... I used github api to get the patch but i did not notice that patch is not a required field for the response from the api and sometimes its missing, i added a fix but now i can't replicate a case where patch field is missing, i hope i will be able to replicate this and open a PR soon.

@calebcartwright
Copy link
Member

Awesome, that looks like some great progress there @jNullj!

@jNullj
Copy link
Collaborator

jNullj commented Aug 11, 2023

I changed this, it appears to be way more robust to just use the two package-lock.json files to extract the version. It is also way more robust, as some packages might update a minor version of this package sneakily.

I will PR soon, I am trying to find where this fails.
image

I made it so far so this warring will show even if no override was done to changed components, should i change it to hide the message if non of the components we look for did not change? or should i keep it with an empty list?

@chris48s
Copy link
Member Author

This looks great based on the screenshots 😄 I think it is probably easier to follow up on the questions in the context of reviewing the PR now that you have opened that. I'll have a look over the weekend. Thanks

github-merge-queue bot pushed a commit that referenced this issue Aug 18, 2023
* devops: Add ci warning for swizzled docusaurus components

Add new action and workflow that runs on new dependabot PR.
The action checks changes in components of docusaurus-theme-openapi  that are swizzled and warns maintainers to review those changes.
For more info see #9287

Solves #9287

* Fix table formating

* Fix order

* handle missing patch cases

* fix missing fetch in node16

* fix typo

* fix typo

* changed from diff parse to json usage

* fix keys issues

* fix parent dependency version check

* Apply suggestions from code review - Improve description and text format

Co-authored-by: chris48s <chris48s@users.noreply.github.com>

* Fix comment table format

* fix type

* Add docusaurus-swizzled-warning to dependabot

* refactor: remove node-fetch and use octokit insted

* improve code readability

* fix type

* change pull_request_target to pull_request

* Improve action log

Change core.debug to core.info so it always shows and not only when in debug.

* Log old and new version

Log old and new versions for the Docusaurus swizzled component changes warning workflow.

---------

Co-authored-by: chris48s <chris48s@users.noreply.github.com>
@chris48s
Copy link
Member Author

done in #9467

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Dev tooling, test framework, and CI frontend The React app and the infrastructure that supports it
Projects
None yet
Development

No branches or pull requests

3 participants