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.

Sign up for GitHub

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

Jump to bottom

Fix schema error #61

Merged
merged 6 commits into from
Jul 13, 2023
Merged

Fix schema error #61

merged 6 commits into from
Jul 13, 2023

Conversation

steff456
Copy link
Contributor

@steff456 steff456 commented Jul 3, 2023

Fixes #57

This PR,

  • Removes unnecessary lerna option
  • Removes schema from main package.json
  • Removes schema folder from main package
  • Avoid setting registry inside the plugin of each theme

@steff456 steff456 added the type: bug 🐛 Something isn't working label Jul 3, 2023
@steff456 steff456 requested a review from trallard July 3, 2023 22:42
@steff456 steff456 self-assigned this Jul 3, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023 •
edited
Loading

Binder 👈 Launch a binder notebook on branch Quansight-Labs/jupyterlab-accessible-themes/fix-schema
Comment updated on 2023-07-13T18:51:15.627Z

This was referenced Jul 3, 2023
Closed
Closed
@trallard trallard mentioned this pull request Jul 7, 2023
Merged
4 tasks
@trallard trallard removed their request for review July 7, 2023 14:02
trallard
trallard approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Tested locally and it seems to have fixed the schema - will ask for a review from @gabalafou on the code side

@trallard trallard requested a review from gabalafou July 7, 2023 14:04
gabalafou
gabalafou requested changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

I just have a few questions before I feel comfortable hitting the approve button

lerna.json Outdated Show resolved Hide resolved
packages/githublight/src/index.ts Show resolved Hide resolved
gabalafou
gabalafou approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

I'm going to approve this so you're not blocked, but would you mind removing the workspaces key from lerna.json if you can? (or leave a comment if you can't)

@steff456
Copy link
Contributor Author

I was able to remove those lines. I think this PR is ready for merge!

@steff456 steff456 mentioned this pull request Jul 13, 2023
Closed
@trallard
Copy link
Member

Linting, links, and formatting issues resolved, so ready to merge 🚀

@trallard trallard merged commit 1efd781 into main Jul 13, 2023
@trallard trallard deleted the fix-schema branch July 13, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trallard trallard trallard approved these changes

@gabalafou gabalafou gabalafou approved these changes

Assignees

@steff456 steff456

Labels
status: done ✅ type: bug 🐛 Something isn't working
Projects
jupyter-a11y-tools 🛠
Status: Done 💪🏾
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing schema when changing theme
3 participants
@steff456 @trallard @gabalafou