Add: documentation links for data sources #1381
Conversation
| @@ -110,7 +110,7 @@ <h4 class="modal-title">Query Archive</h4> | |||
| <span class="text-muted">Data Source</span> | |||
| <select ng-disabled="!isQueryOwner" ng-model="query.data_source_id" ng-change="updateDataSource()" | |||
| ng-options="ds.id as ds.name for ds in dataSources"></select> | |||
|
|
|||
| <a ng-href={{dataSource.doc_url}}>{{dataSource.type_name}} documentation</a> | |||
There was a problem hiding this comment.
can you add a screenshot of how it looks like?
There was a problem hiding this comment.
also worth showing only if the url isn't empty.
There was a problem hiding this comment.
Fixed so that it should only show when doc_url is present.
washort
force-pushed
the
data-source-docs
branch
2 times, most recently
from
dd5ca59 to
3aaacad
Compare
| @@ -110,7 +110,7 @@ <h4 class="modal-title">Query Archive</h4> | |||
| <span class="text-muted">Data Source</span> | |||
| <select ng-disabled="!isQueryOwner" ng-model="query.data_source_id" ng-change="updateDataSource()" | |||
| ng-options="ds.id as ds.name for ds in dataSources"></select> | |||
|
|
|||
| <a ng-if="dataSource.options.doc_url" ng-href={{dataSource.options.doc_url}}>{{dataSource.type_name}} documentation</a> | |||
There was a problem hiding this comment.
As I'm not sure how I feel about this, can you hide it behind a feature flag (with default being off)?
(check how I did it for the Manage Permissions button)
Thanks.
arikfr
changed the title
washort
force-pushed
the
data-source-docs
branch
from
3aaacad to
7a1db84
Compare
washort
force-pushed
the
data-source-docs
branch
from
7a1db84 to
f976cb2
Compare
|
Coming to thin of it -- why do we need this as a configuration value for each data source? |
washort
force-pushed
the
data-source-docs
branch
from
f976cb2 to
b16819d
Compare
|
The reason for configuration values is so that individual data sources can link to data-specific docs rather than just the language reference. (Or to the version of the docs matching the backend DB version, etc.) |
washort
force-pushed
the
data-source-docs
branch
from
b16819d to
308a962
Compare
washort
force-pushed
the
data-source-docs
branch
2 times, most recently
from
4a4e489 to
745cde7
Compare
washort
force-pushed
the
data-source-docs
branch
from
745cde7 to
a655131
Compare
|
Hi! Just following up on this. We'd really like to get this deployed for our local installation within the next week. Is there a chance of getting this PR merged upstream soon, or should we add it to our own fork for now? Thanks for your attention! |
|
Let me review this again on Sunday and get back to you. |
|
I don't want to make it hard on you to maintain your fork, but on the other hand don't feel 100% with merging this. As even when it's behind a feature flag, it introduces code that we will have to maintain that won't benefit the general users population. Let's keep this on your fork for now, and if we see it makes syncing with upstream a hassle, will revisit to think what we can do about this. How's that sound? |
|
We're always going to have some stuff that needs to be in our own fork. I don't think this will be a burden for now. Thanks for having a look. |
Add a customizable link to documentation for a data source, defaulting to the reference docs for the query language for that source.