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

Update where app-model actions/providers/processors live in codebase #416

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

lucyleeow
Copy link
Collaborator

@lucyleeow lucyleeow commented May 2, 2024

References and relevant issues

Relates to napari/napari#6848
Depends on napari/napari#6743

Description

Updates the info on where actions live in the napari codebase.

@lucyleeow
Copy link
Collaborator Author

cc @DragaDoncila

@lucyleeow lucyleeow added this to the 0.5.0 milestone May 2, 2024
@github-actions github-actions bot added the documentation Improvements or additions to documentation label May 2, 2024
Copy link
Contributor

@DragaDoncila DragaDoncila left a comment

Choose a reason for hiding this comment

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

Thanks @lucyleeow just some nits here

docs/developers/architecture/app_model.md Outdated Show resolved Hide resolved
docs/developers/architecture/app_model.md Outdated Show resolved Hide resolved
of living in a menu and are thus considered 'Qt' actions.
This also ensures that there is only one file defining.

The layer context menu actions do not require a GUI and thus live in
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The layer context menu actions do not require a GUI and thus live in
The layer context menu actions do not require a GUI (they only require the `layerlist`) and thus live in

@lucyleeow
Copy link
Collaborator Author

Thanks, changes made!

Copy link
Contributor

@DragaDoncila DragaDoncila left a comment

Choose a reason for hiding this comment

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

Awesome sauce, thanks @lucyleeow!

@lucyleeow
Copy link
Collaborator Author

AH whoops sorry, I forgot this has already been approved.

I realise that we now no longer have qt providers or processors anymore, so I've updated that here too. Sorry!

@lucyleeow lucyleeow changed the title Update where app-model actions live in napari codebase Update where app-model actions/providers/processors live in napari codebase May 3, 2024
@lucyleeow lucyleeow changed the title Update where app-model actions/providers/processors live in napari codebase Update where app-model actions/providers/processors live in codebase May 3, 2024
@DragaDoncila
Copy link
Contributor

@lucyleeow do we now have to wait for #6743 to go in before we merge this?

@lucyleeow
Copy link
Collaborator Author

🤦 yes you are right. I'll add a depends on

@Czaki
Copy link
Contributor

Czaki commented May 3, 2024

test comment

@lucyleeow
Copy link
Collaborator Author

The depends on has now been merged (not sure why the CI is still failing) so maybe this could go in now, @DragaDoncila ?

@DragaDoncila DragaDoncila merged commit ef267d3 into napari:main Jun 7, 2024
8 checks passed
@DragaDoncila
Copy link
Contributor

thanks @lucyleeow sorry for the delay...

@lucyleeow lucyleeow deleted the app-actions branch June 7, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants