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

Expose syntax_style #261

Merged
merged 3 commits into from
Dec 16, 2022
Merged

Expose syntax_style #261

merged 3 commits into from
Dec 16, 2022

Conversation

brisvag
Copy link
Contributor

@brisvag brisvag commented Dec 14, 2022

While working on fixing napari themes (see this zulip conversation) I noticed that some fields that are available in napari are not exposed/tested here: icon was unused, and syntax_style was not exposed. I added them to the specs and description.

Also, I noticed that if you pass wrong/nonexisting keys, npe2 does not complain. Is this intended?

@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Merging #261 (f82d5dd) into main (0db5c3e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #261   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         2806      2807    +1     
=========================================
+ Hits          2806      2807    +1     
Impacted Files Coverage Δ
npe2/manifest/contributions/_themes.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tlambert03
Copy link
Collaborator

if you pass wrong/nonexisting keys, npe2 does not complain. Is this intended?

Yes, otherwise every new field we add to the schema becomes a breaking change for anyone on an older version of npe2

@brisvag
Copy link
Contributor Author

brisvag commented Dec 14, 2022

Makes sense. It would be nice if the npe2 validate command could tell you that you have keys in your schema that do not exist in the current npe2 version. Would help with debugging!

@brisvag
Copy link
Contributor Author

brisvag commented Dec 14, 2022

I can't tell if this failure is a real bug. Are we missing napari-svg for macos?

@tlambert03
Copy link
Collaborator

just an internet problem

@brisvag
Copy link
Contributor Author

brisvag commented Dec 14, 2022

Still failing :/

@tlambert03
Copy link
Collaborator

still think it's internet... see this test: https://github.com/napari/npe2/actions/runs/3693708290/jobs/6256673381 where everything passed but codecov

@tlambert03
Copy link
Collaborator

see #255

Copy link
Collaborator

@nclack nclack left a comment

Choose a reason for hiding this comment

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

lgtm! Just updated to fix the tests. If they all pass now, I'll look to merge tomorrow.

@nclack nclack merged commit 367b31f into napari:main Dec 16, 2022
@kne42 kne42 added the enhancement New feature or request label Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants