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 apache_arrow for status command #1384

Merged
merged 10 commits into from
Aug 23, 2022

Conversation

HashidaTKS
Copy link
Contributor

@HashidaTKS HashidaTKS commented Aug 10, 2022

@HashidaTKS HashidaTKS force-pushed the doc_add_status_apache_arrow branch 2 times, most recently from 8173c53 to 27cd8d3 Compare August 10, 2022 09:59
* - ``apache_arrow``
- .. versionadded:: 10.0.1

The version of Apache Arrow that Groonga uses. This item only displays when Apache Arrow is valid in Groonga.
Copy link
Member

Choose a reason for hiding this comment

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

valid: enabled or supported?

@kou
Copy link
Member

kou commented Aug 16, 2022

@yoshimotoyuk Could you review this?

@HashidaTKS HashidaTKS marked this pull request as ready for review August 18, 2022 02:36
@HashidaTKS
Copy link
Contributor Author

@kou Would you re-review this when you have time?
@yoshimotoyuk Would you review English of this change when you have time?

"version": VERSION
"version": VERSION,
"features": FEATURES,
"apache_arrow": APACHE_ARROW_VERSION
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"apache_arrow": APACHE_ARROW_VERSION
"apache_arrow": APACHE_ARROW_INFORMATION

We may add more information later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I have fixed it.

* - ``apache_arrow``
- .. versionadded:: 10.0.1

The version of Apache Arrow that Groonga uses. This item displays only when Apache Arrow is enabled in Groonga.
Copy link
Member

Choose a reason for hiding this comment

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

We may add not only version information but also more information later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I have fixed it.

msgstr ""

msgid ""
"The list of features that Groonga has and whether they are enabled or not."
Copy link
Contributor

@yoshimotoyuk yoshimotoyuk Aug 23, 2022

Choose a reason for hiding this comment

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

Suggested change
"The list of features that Groonga has and whether they are enabled or not."
"The list of Groonga's features and status(enabled or distabled)"

Above suggestion for simple description.
If detailed description is preferred as follows;

"The list shows features of Groonga and whether they are enabled or disabled."

When you used " enabled or not " as a adjective to explain status it would be better to say " enabled or disabled".

Copy link
Contributor

Choose a reason for hiding this comment

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

もし、リストに有効なものだけが出るというのであれば、言い方を変えたほうがいいので教えてください。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます。リストには有効なものと無効なものどちらも出るので、この案でいかせてもらいます

* - ``features``
- .. versionadded:: 10.0.1

The list of features that Groonga has and whether they are enabled or not.
Copy link
Contributor

Choose a reason for hiding this comment

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

is it common sense that true means "enabled" and false means "disabled"?
This is just my question...

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 think, for developers or users of Groonga, it is common sense, or at least it makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is very interesting. I shall remember! thank you.

Copy link
Contributor

@yoshimotoyuk yoshimotoyuk left a comment

Choose a reason for hiding this comment

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

Reviewing based on the condition that simple and short description would be preferred.

@HashidaTKS
Copy link
Contributor Author

Thanks, I have reflected the feedback.

HashidaTKS and others added 3 commits August 23, 2022 15:49
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
msgstr ""

msgid ""
"The version information of Apache Arrow that Groonga currently uses. It's "
Copy link
Contributor Author

Choose a reason for hiding this comment

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

和訳の更新を忘れていたのですが、将来的にバージョン以外の情報も入るのでThe information about ...に戻そうと思います。

@HashidaTKS
Copy link
Contributor Author

Thank you.
I have reflected the feedback.

@kou kou merged commit d9d8fe3 into groonga:master Aug 23, 2022
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