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

Add news 12.0.9 #1434

Merged
merged 22 commits into from
Oct 28, 2022
Merged

Add news 12.0.9 #1434

merged 22 commits into from
Oct 28, 2022

Conversation

HashidaTKS
Copy link
Contributor

No description provided.

@HashidaTKS
Copy link
Contributor Author

@kou @komainu8

Would you review this when you have time?
I intentionally omitted dbe5549 because it has only a minimum implementation and it is not expected to be used by users yet.

@yoshimotoyuk

Would you review this about English when you have time?

------------

* [:doc:`reference/functions/escalate`] Added a document for the ``escalate()`` function.

Copy link
Member

Choose a reason for hiding this comment

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

We need to add 58e49dd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

------------

* [:doc:`reference/functions/escalate`] Added a document for the ``escalate()`` function.

Copy link
Member

Choose a reason for hiding this comment

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

We need to add 0c1280c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

This had occured when using :ref:`select-n-workers` with a value greater than ``1`` and ``drilldowns[{LABEL}].filter`` at the same time.

This was because referencing incorrect values when performing internal parallel processing.
So if the condition above was satisfied, Groonga sometimes crashed depending on the timing of the parallel processing.
Copy link
Member

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.

理屈から考えて不正な結果を返す場合もありそうだったので、その旨も記載しました。

Copy link
Member

@komainu8 komainu8 left a comment

Choose a reason for hiding this comment

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

Could you confirm my comments?

@HashidaTKS
Copy link
Contributor Author

@komainu8

Thanks, I have addressed your comments.

@kou
Copy link
Member

kou commented Oct 27, 2022

I intentionally omitted dbe5549 because it has only a minimum implementation and it is not expected to be used by users yet.

Could you list it as an experimental feature?

@HashidaTKS
Copy link
Contributor Author

@kou

Could you list it as an experimental feature?

Sure, I have added a description about NormalizerHTML as an experimental feature.

This was because referencing incorrect values (objects) when performing internal parallel processing.
So if the condition above was satisfied, Groonga sometimes crashed or returned incorrect results depending on the timing of the parallel processing.

* Suppressed logging a lot of same messages when no memory available.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is not a bug.
Because it is not abnormaly that Groonga output a lot of same messages when no memory available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to Improvement.


In this sample, ``&`` and ``&`` are unescaped to ``&``.

* [httpd] Updated bundled nginx to 1.23.2.
Copy link
Member

Choose a reason for hiding this comment

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

This update include security bug fix.
Therefore, we should also add about security bug fix.

See: https://nginx.org/en/CHANGES

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.

日本語の「これは」の感覚で「This」に置き換えると少し違和感があります。
どのThis?という気持になるので「この機能」や「この修正」などにしたほうが分かりやすくなると思います。


* [:doc:`reference/normalizers`] Added NormalizerHTML. (Experimental)

This is a normalizer for HTML.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of "This", the subject would be better to be more explaining such as this function or normalizerHTML, self.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

doc/source/news.rst Outdated Show resolved Hide resolved
doc/source/news.rst Outdated Show resolved Hide resolved

This had occured when using :ref:`select-n-workers` with a value greater than ``1`` and ``drilldowns[{LABEL}].filter`` at the same time.

This was because referencing incorrect values (objects) when performing internal parallel processing.
Copy link
Contributor

Choose a reason for hiding this comment

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

same here when using " this " as the subject it would be nice to add the main word.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@HashidaTKS
Copy link
Contributor Author

日本語の「これは」の感覚で「This」に置き換えると少し違和感があります。

完全に日本語の「これは」の感覚で使っていたので、ご指摘の通り修正しました。

@HashidaTKS
Copy link
Contributor Author

@kou @komainu8 @yoshimotoyuk

Thank you, I have addressed your comments.

@komainu8 komainu8 merged commit 6c3a823 into groonga:master Oct 28, 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

4 participants