You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "317brian (via GitHub)" <gi...@apache.org> on 2023/02/22 22:15:27 UTC
[GitHub] [druid] 317brian opened a new pull request, #13835: docs: fix html nits
317brian opened a new pull request, #13835:
URL: https://github.com/apache/druid/pull/13835
### Description
Really minor fixes like closing the <br /> tag in preparation for Docusaurus2, which doesn't like it when they're not closed.
This PR has:
- [x] been self-reviewed.
--
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] 317brian commented on pull request #13835: docs: fix html nits
Posted by "317brian (via GitHub)" <gi...@apache.org>.
317brian commented on PR #13835:
URL: https://github.com/apache/druid/pull/13835#issuecomment-1440894118
Yeah, someone else was running into this earlier. It passes spellcheck locally, but for some reason the GHA doesn't like the position of the comma. It wants it outside of the `, which is technically correct but looks odd in the rendered output.
--
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] 317brian commented on pull request #13835: docs: fix html nits
Posted by "317brian (via GitHub)" <gi...@apache.org>.
317brian commented on PR #13835:
URL: https://github.com/apache/druid/pull/13835#issuecomment-1442544662
The step in CI that's failing is the license check
```
Error: found 1 missing licenses. These licenses are reported, but missing in the registry
druid_module: avro-extensions, groupId: com.sun.activation, artifactId: jakarta.activation, version: 1.2.1, license: Eclipse Distribution License 1.0
```
It's unlikely to be caused by this PR since this PR only touches files in `docs` and not the avro-extension. The PR also doesn't add any new files that would need a license.
--
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] jwitko commented on pull request #13835: docs: fix html nits
Posted by "jwitko (via GitHub)" <gi...@apache.org>.
jwitko commented on PR #13835:
URL: https://github.com/apache/druid/pull/13835#issuecomment-1440888225
Hey @317brian I have https://github.com/apache/druid/pull/13747 failing on a static CI check for something that looks appropriate for this PR and I'm guessing it will fail for you as well?
```
> spellcheck
> mdspell --en-us --ignore-numbers --report '../docs/**/*.md' || (./script/notify-spellcheck-issues && false)
../docs/tutorials/tutorial-jupyter-index.md
41 | er both try to use port `8888,` so start Jupyter on a different port
>> 1 spelling error found in 227 files
```
Although to be fair I have no idea what its upset about? Seems to be complaining about [this line](https://github.com/apache/druid/blob/master/docs/tutorials/tutorial-jupyter-index.md?plain=1#L41)
--
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] paul-rogers commented on a diff in pull request #13835: docs: fix html nits
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on code in PR #13835:
URL: https://github.com/apache/druid/pull/13835#discussion_r1116316662
##########
docs/tutorials/tutorial-kafka.md:
##########
@@ -45,7 +45,7 @@ Before you follow the steps in this tutorial, download Druid as described in the
```
2. If you're already running Kafka on the machine you're using for this tutorial, delete or rename the `kafka-logs` directory in `/tmp`.
- > Druid and Kafka both rely on [Apache ZooKeeper](https://zookeeper.apache.org/) to coordinate and manage services. Because Druid is already running, Kafka attaches to the Druid ZooKeeper instance when it starts up.<br>
+ > Druid and Kafka both rely on [Apache ZooKeeper](https://zookeeper.apache.org/) to coordinate and manage services. Because Druid is already running, Kafka attaches to the Druid ZooKeeper instance when it starts up.<br />
Review Comment:
No need for the `<br/>`, but no harm either.
--
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] paul-rogers commented on a diff in pull request #13835: docs: fix html nits
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on code in PR #13835:
URL: https://github.com/apache/druid/pull/13835#discussion_r1115968640
##########
docs/ingestion/ingestion-spec.md:
##########
@@ -477,7 +477,7 @@ The `indexSpec` object can include the following properties:
|-----|-----------|-------|
|bitmap|Compression format for bitmap indexes. Should be a JSON object with `type` set to `roaring` or `concise`. For type `roaring`, the boolean property `compressRunOnSerialization` (defaults to true) controls whether or not run-length encoding will be used when it is determined to be more space-efficient.|`{"type": "roaring"}`|
|dimensionCompression|Compression format for dimension columns. Options are `lz4`, `lzf`, `zstd`, or `uncompressed`.|`lz4`|
-|stringDictionaryEncoding|Encoding format for STRING value dictionaries used by STRING and COMPLEX<json> columns. <br>Example to enable front coding: `{"type":"frontCoded", "bucketSize": 4}`<br>`bucketSize` is the number of values to place in a bucket to perform delta encoding. Must be a power of 2, maximum is 128. Defaults to 4.<br>See [Front coding](#front-coding) for more information.|`{"type":"utf8"}`|
+|stringDictionaryEncoding|Encoding format for STRING value dictionaries used by STRING and COMPLEX<json> columns. <br />Example to enable front coding: `{"type":"frontCoded", "bucketSize": 4}`<br />`bucketSize` is the number of values to place in a bucket to perform delta encoding. Must be a power of 2, maximum is 128. Defaults to 4.<br />See [Front coding](#front-coding) for more information.|`{"type":"utf8"}`|
Review Comment:
Breaks here do seem to be needed: at least the VS Code preview gets confused with newlines within a table.
Elsewhere in this table we use two breaks to create a clear separation between paragraphs. Should we do that here as well for consistency? The resulting text looks better.
##########
docs/operations/rule-configuration.md:
##########
@@ -153,9 +153,9 @@ Set the following properties:
- `period`: a JSON object representing [ISO-8601](https://en.wikipedia.org/wiki/ISO_8601) periods. The period is from some time in the past to the present, or into the future if `includeFuture` is set to `true`.
- `includeFuture`: a boolean flag to instruct Druid to match a segment if:
-<br>- the segment interval overlaps the rule interval, or
-<br>- the segment interval starts any time after the rule interval starts.
-<br>You can use this property to load segments with future start and end dates, where "future" is relative to the time when the Coordinator evaluates data against the rule. Defaults to `true`.
+<br />- the segment interval overlaps the rule interval, or
+<br />- the segment interval starts any time after the rule interval starts.
+<br />You can use this property to load segments with future start and end dates, where "future" is relative to the time when the Coordinator evaluates data against the rule. Defaults to `true`.
Review Comment:
At the cost of extra per-line spacing, you can replace the `<br />` above with two spaces and get the same effect in a more Markdown-ish way. Here and below.
##########
docs/ingestion/migrate-from-firehose-ingestion.md:
##########
@@ -53,7 +53,7 @@ Edit the new file as follows:
4. Move the `format` definition from `parser.parseSpec` to an `inputFormat` definition in `ioConfig`.
5. Delete the `parser` definition.
6. Save the file.
-<br>You can check the format of your new ingestion file against the [migrated example](#example-ingestion-spec-after-migration) below.
+<br />You can check the format of your new ingestion file against the [migrated example](#example-ingestion-spec-after-migration) below.
Review Comment:
Break here isn't really needed. Reads better without it.
##########
docs/tutorials/tutorial-kafka.md:
##########
@@ -45,7 +45,7 @@ Before you follow the steps in this tutorial, download Druid as described in the
```
2. If you're already running Kafka on the machine you're using for this tutorial, delete or rename the `kafka-logs` directory in `/tmp`.
- > Druid and Kafka both rely on [Apache ZooKeeper](https://zookeeper.apache.org/) to coordinate and manage services. Because Druid is already running, Kafka attaches to the Druid ZooKeeper instance when it starts up.<br>
+ > Druid and Kafka both rely on [Apache ZooKeeper](https://zookeeper.apache.org/) to coordinate and manage services. Because Druid is already running, Kafka attaches to the Druid ZooKeeper instance when it starts up.<br />
Review Comment:
This reads better as:
```text
> Druid and Kafka both rely on ...
>
> In a production environment ...
```
--
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] abhishekrb19 commented on pull request #13835: docs: fix html nits
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on PR #13835:
URL: https://github.com/apache/druid/pull/13835#issuecomment-1441275095
@317brian @jwitko, yeah, I ran into the same spellcheck failure. `spellcheck` assumed there was a huge codeblock with an incorrect syntax because of an additional backtick that got accidentally introduced. It's now fixed in master; if you pull the top of master, that should have the fix https://github.com/apache/druid/pull/13836.
--
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] vtlim merged pull request #13835: docs: fix html nits
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim merged PR #13835:
URL: https://github.com/apache/druid/pull/13835
--
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