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

doc: Add description about drilldown.table #1385

Merged
merged 19 commits into from
Aug 31, 2022

Conversation

HashidaTKS
Copy link
Contributor

drilldown.table is not documented.

@HashidaTKS HashidaTKS marked this pull request as ready for review August 24, 2022 08:42
@HashidaTKS HashidaTKS requested a review from kou August 24, 2022 08:42
table_create Tags TABLE_PAT_KEY ShortText
# [
# [
# -22,
Copy link
Member

Choose a reason for hiding this comment

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

Is this expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to resolve this...
Maybe this is because another example in select.rst already creates Tags table...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have renamed Tags to NestedDrilldownTags and Memos to NestedDrilldownMemos.


.. versionadded:: 6.0.2

Specifies ``${LABLE}`` of other ``drilldown``, ``drilldowns`` or ``slices``.
Copy link
Member

Choose a reason for hiding this comment

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

Can we specify ${LABEL} for drilldown? drilldown doesn't have its label.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I thought we can use drilldown[${LABEL}] syntax, but now I notice it is deprecated, so I will remove drilldown from this line.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this sentence is a imperative form. And in that case, verb should be 現在形.
so would be "specify" not "specifies".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have fixed it.

.. {"_key": "Rroonga", "category": "Ruby"}
.. ]
.. select Memos \
.. --drilldowns[label_tag].keys tag \
Copy link
Member

Choose a reason for hiding this comment

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

Can we use better name for label_tag?
I don't want users to use label_XXX for label.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed to simply use Tag and Category for these drilldowns table.

@kou
Copy link
Member

kou commented Aug 25, 2022

@yoshimotoyuk Could you review this?


msgid ""
"The result of the specified ``${LABLE}`` is drilldowned, which means that "
"this paramter enables nested aggregate calculations and groups in drilldown."
Copy link
Member

Choose a reason for hiding this comment

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

「このパラメーターを使えば多段ドリルダウンできます」とかもっとシンプルな言い回しにしませんか?

HashidaTKS and others added 7 commits August 29, 2022 14:26
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Co-authored-by: Yukiko Yoshimoto <86996282+yoshimotoyuk@users.noreply.github.com>
Co-authored-by: Yukiko Yoshimoto <86996282+yoshimotoyuk@users.noreply.github.com>
Use breking up lines
@HashidaTKS
Copy link
Contributor Author

「このパラメーターを使えば多段ドリルダウンできます」とかもっとシンプルな言い回しにしませんか?

そのように修正しました。

@HashidaTKS
Copy link
Contributor Author

@yoshimotoyuk
Would you re-review English for this patch when you have time?

Co-authored-by: Yukiko Yoshimoto <86996282+yoshimotoyuk@users.noreply.github.com>
@yoshimotoyuk
Copy link
Contributor

overall looked nice to me otherwise. :)

@HashidaTKS
Copy link
Contributor Author

@yoshimotoyuk

Thank you!

@kou

Would you re-review this when you have time?

@kou kou merged commit a5c78cf into groonga:master Aug 31, 2022
@kou
Copy link
Member

kou commented Aug 31, 2022

Thanks.

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.

None yet

3 participants