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 return code: add the detail of the error code #1369

Merged
merged 1 commit into from
Jul 1, 2022
Merged

Conversation

komainu8
Copy link
Member

No description provided.

msgstr "最初に、参照元のテーブルやカラムを削除します。"

msgid "Secondly, we remove a table of reference destinations."
msgstr "次に、参照元のテーブルを削除します。"
Copy link

Choose a reason for hiding this comment

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

Suggested change
msgstr "次に、参照元のテーブルを削除します。"
msgstr "次に、参照先のテーブルを削除します。"

そもそも、英語もそうですが、「参照先のテーブル」というより、
table_remove で削除したかったテーブル」なんですよね。
確かに繰り返すのもくどくて、良い表現がぱっと出てこないですが、
例えばdatabase_unmapの説明のように書いた方が、わかりやすかったりしないですかね。

@daipom
Copy link

daipom commented Jun 17, 2022

エラーコードに対して考えうる原因と対応策が簡潔にまとまっているのは、とても有難いことだと思います!

原因と対応策に焦点を当てた簡潔な構成で、とても分かりやすいと思いました。

この規模であれば、原因と対応策を分離して書くのではなくて、
今の対応策の部分だけあれば十分なのではないかとちょっと思いましたが(結局対応策に原因も並んでいるので)、
まず原因だけ調べたいというケースも多そうなので、今の構成でも良いと思いました。

@daipom
Copy link

daipom commented Jun 17, 2022

Secondly, ...の部分はなくても良いかもしれないですね。
Firstly, ...の部分が対応策だと思うので、それだけあれば良いかなあとは思います。
以下のようなイメージ。

1. If the table that is a target of ``table_remove`` has reference from other table and column.

   We have to remove tables and columns of the reference sources.

@@ -0,0 +1,30 @@
.. -*- rst -*-

``GRN_OPERATION_NOT_PERMITTED``
Copy link
Contributor

@HashidaTKS HashidaTKS Jun 17, 2022

Choose a reason for hiding this comment

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

How about also adding the error code number like below?
I think users will look for information based on the error code number because queries return the number.

-2: GRN_OPERATION_NOT_PERMITTED

@komainu8 komainu8 marked this pull request as ready for review June 24, 2022 06:46
@komainu8
Copy link
Member Author

I'll merge this PR if all CI pass.

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