-
Notifications
You must be signed in to change notification settings - Fork 11
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
Define omero.qa.feedback as new visible server property #38
Merged
+11
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't read through the rest of the PR, but in terms of symmetry "omero.feedback.url" rings truer for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have
omero.upgrades.url
soomero.feedback.url
oromero.qa.url
will make senseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting thoughts. I went for the minimal amount of changes/extra configuration but happy to update to use a different property name if we reach a consensus.
After a bit of research, a few comments on QA vs feedback:
OMERO.qa
is the historical name of the OME application which was built to receive the feedback from clients and handle upgrade checks - see https://omero.readthedocs.io/en/stable/users/history.html#beta-4-1-october-2009 for the original announcementomero_qa
namespace is primarily used internally within the appomero.qa.feedback
OMERO.web propertyomero.web.feedback.*
settings allow to turn on/off part of the feedback mechanismomero.feedback.url
, I suspect we would eventually try to deprecateomero.qa.feedback
and update the client to consumeomero.feedback.url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that summary, @sbesson. The fact that it's also documentation in sysadmin/config.rst leads me to say let's not change it. (At least in terms of the naming, I feel somewhat vindicated by the environment variable in https://github.com/ome/omero-web/blob/3ae446b01d98493b109998c1dece8224337db54a/omeroweb/settings.py#L267 😄)