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

omero render set: clarify help around settings for disabled channels #67

Merged
merged 4 commits into from
Nov 27, 2024

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Nov 15, 2024

Closes #66

The underlying code change has been applied in #43 and released since omero-cli-render 0.6.0 but the help was never updated and was still documenting the previous behavior.

src/omero_cli_render.py Outdated Show resolved Hide resolved
src/omero_cli_render.py Outdated Show resolved Hide resolved
Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

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

Thank you @sbesson - please see 2 reformulation suggestions. To be fair, I did understand your formulation too.

@sbesson
Copy link
Member Author

sbesson commented Nov 15, 2024

Thanks @pwalczysko slightly reformulated the second sentence to make it more grammatically correct. Can you take another look?

Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

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

Yes, I cannot find a fault, both understanding and grammar seems +1 to me. Thanks

@jburel jburel closed this Nov 21, 2024
@jburel jburel reopened this Nov 21, 2024
@jburel
Copy link
Member

jburel commented Nov 21, 2024

flake8.main.application   MainProcess    623 INFO     Found a total of 12 violations and reported 1
./src/omero_cli_render.py:125:78: W291 trailing whitespace

Copy link
Member

@jburel jburel left a comment

Choose a reason for hiding this comment

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

Release as 0.8.2?

@sbesson
Copy link
Member Author

sbesson commented Nov 21, 2024

I think there is harm in releasing the package with the updated help message. Possibly depends on whether there are other candidates for inclusion.

@jburel
Copy link
Member

jburel commented Nov 21, 2024

Nothing really in the pipeline as far as I can tell
cc @dominikl

Copy link
Member

@dominikl dominikl left a comment

Choose a reason for hiding this comment

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

👍

@dominikl dominikl merged commit c0517ed into ome:master Nov 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

in set command, inactive channels' settings are not set
4 participants