You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/08/24 06:09:25 UTC
[GitHub] [druid] bananaaggle opened a new pull request #11628: Fix TRIM help broken in Console SQL Editor
bananaaggle opened a new pull request #11628:
URL: https://github.com/apache/druid/pull/11628
<!-- Thanks for trying to help us make Apache Druid be the best it can be! Please fill out as much of the following information as is possible (where relevant, and remove it when irrelevant) to help make the intention and scope of this PR clear in order to ease review. -->
<!-- Please read the doc for contribution (https://github.com/apache/druid/blob/master/CONTRIBUTING.md) before making this PR. Also, once you open a PR, please _avoid using force pushes and rebasing_ since these make it difficult for reviewers to see what you've changed in response to their reviews. See [the 'If your pull request shows conflicts with master' section](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master) for more details. -->
Fixes [11625](https://github.com/apache/druid/issues/11625). One escape cause this trouble.
<!-- Replace XXXX with the id of the issue fixed in this PR. Remove this section if there is no corresponding issue. Don't reference the issue in the title of this pull-request. -->
<!-- If you are a committer, follow the PR action item checklist for committers:
https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers. -->
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11628:
URL: https://github.com/apache/druid/pull/11628#issuecomment-905942359
thanks for looking into this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle commented on pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
bananaaggle commented on pull request #11628:
URL: https://github.com/apache/druid/pull/11628#issuecomment-906084173
> backticks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle commented on pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
bananaaggle commented on pull request #11628:
URL: https://github.com/apache/druid/pull/11628#issuecomment-906085284
> Are the backticks necessary in the docs? I think we want the hint to show up as `TRIM([BOTH | LEADING | TRAILING] [FROM] expr)`
Actually, backtick appear in description is not necessary too. Just like:
<img width="517" alt="Screen Shot 2021-08-26 at 11 44 49 AM" src="https://user-images.githubusercontent.com/24642075/130900539-cc6d8ab9-0448-48c0-9b3e-cfc687314af2.png">
I delete it in this PR. Now it displayed like this:
<img width="512" alt="Screen Shot 2021-08-26 at 12 23 26 PM" src="https://user-images.githubusercontent.com/24642075/130900604-834197df-da03-414e-bc28-ef596a5336f3.png">
<img width="510" alt="Screen Shot 2021-08-26 at 12 23 11 PM" src="https://user-images.githubusercontent.com/24642075/130900607-dfe88616-c65a-4db5-aede-6496639c86a7.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle merged pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
bananaaggle merged pull request #11628:
URL: https://github.com/apache/druid/pull/11628
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s edited a comment on pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
suneet-s edited a comment on pull request #11628:
URL: https://github.com/apache/druid/pull/11628#issuecomment-906836158
> > Are the backticks necessary in the docs? I think we want the hint to show up as `TRIM([BOTH | LEADING | TRAILING] [FROM] expr)`
>
> Actually, backtick appear in description is not necessary too. Just like:
> <img alt="Screen Shot 2021-08-26 at 11 44 49 AM" width="517" src="https://user-images.githubusercontent.com/24642075/130900539-cc6d8ab9-0448-48c0-9b3e-cfc687314af2.png">
I actually think we want these backticks as it's showcasing it is a function in the description and not meant to be grammatically correct
> I delete it in this PR. Now it displayed like this:
> <img alt="Screen Shot 2021-08-26 at 12 23 26 PM" width="512" src="https://user-images.githubusercontent.com/24642075/130900604-834197df-da03-414e-bc28-ef596a5336f3.png">
> <img alt="Screen Shot 2021-08-26 at 12 23 11 PM" width="510" src="https://user-images.githubusercontent.com/24642075/130900607-dfe88616-c65a-4db5-aede-6496639c86a7.png">
Overall this is a small thing - and I think this PR makes the hints better so I've approved the change. If it is easy to remove the backticks from the title, but not around a function - like in the example above; I personally think that would be better - but again it's not a blocker, so feel free to merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle closed pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
bananaaggle closed pull request #11628:
URL: https://github.com/apache/druid/pull/11628
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11628:
URL: https://github.com/apache/druid/pull/11628#issuecomment-905942254
Are the backticks necessary in the docs? I think we want the hint to show up as `TRIM([BOTH | LEADING | TRAILING] [FROM] expr)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11628: Fix TRIM help broken in Console SQL Editor
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11628:
URL: https://github.com/apache/druid/pull/11628#issuecomment-906836158
> > Are the backticks necessary in the docs? I think we want the hint to show up as `TRIM([BOTH | LEADING | TRAILING] [FROM] expr)`
>
> Actually, backtick appear in description is not necessary too. Just like:
> <img alt="Screen Shot 2021-08-26 at 11 44 49 AM" width="517" src="https://user-images.githubusercontent.com/24642075/130900539-cc6d8ab9-0448-48c0-9b3e-cfc687314af2.png">
I actually think we want these backticks as it's showcasing it is a function in the description and not meant to be grammatically correct
> I delete it in this PR. Now it displayed like this:
> <img alt="Screen Shot 2021-08-26 at 12 23 26 PM" width="512" src="https://user-images.githubusercontent.com/24642075/130900604-834197df-da03-414e-bc28-ef596a5336f3.png">
> <img alt="Screen Shot 2021-08-26 at 12 23 11 PM" width="510" src="https://user-images.githubusercontent.com/24642075/130900607-dfe88616-c65a-4db5-aede-6496639c86a7.png">
Overall this is a small thing - and I think this PR makes the hints better so I've approved the change. If it is easy to remove the backticks from the title, but not around a function - like in the example above; I personally think that would be better - but again it's not a blocker, so feel free to merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org