Skip to content

Refine description of "About" and mention transparency. - #19716

Merged
coqbot-app[bot] merged 1 commit into
rocq-prover:masterfrom
jfehrle:about_transparency
Nov 4, 2024
Merged

Refine description of "About" and mention transparency.#19716
coqbot-app[bot] merged 1 commit into
rocq-prover:masterfrom
jfehrle:about_transparency

Conversation

@jfehrle

@jfehrle jfehrle commented Oct 18, 2024

Copy link
Copy Markdown
Member

I also edited the text describing the transparency states to make them match the description in the Strategy command. Let me know if I guessed wrong.

@jfehrle jfehrle added the kind: documentation Additions or improvement to documentation. label Oct 18, 2024
@jfehrle jfehrle added this to the 8.20.1 milestone Oct 18, 2024
@jfehrle
jfehrle requested a review from a team as a code owner October 18, 2024 00:13
@coqbot-app coqbot-app Bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Oct 18, 2024
@jfehrle
jfehrle force-pushed the about_transparency branch from b7ed70a to 25c7423 Compare October 18, 2024 01:08
@jfehrle
jfehrle requested a review from a team as a code owner October 18, 2024 01:08
@jfehrle
jfehrle force-pushed the about_transparency branch from 25c7423 to 0b738e9 Compare October 18, 2024 01:42
@SkySkimmer

Copy link
Copy Markdown
Contributor

cc @herbelin

@jfehrle

jfehrle commented Oct 25, 2024

Copy link
Copy Markdown
Member Author

@coq/doc-maintainers Would someone review?

@herbelin

Copy link
Copy Markdown
Member

Regarding the transparency message, I think it is ok as the PR does (this will be adapted anyway in #19029)

Regarding the "expansion weight" lines, which I don't know how it works, I have no idea whether the change is worth or not.

@jfehrle

jfehrle commented Oct 26, 2024

Copy link
Copy Markdown
Member Author

Regarding the "expansion weight" lines, which I don't know how it works, I have no idea whether the change is worth or not.

Does this help?

Conv_oracle fields used in prettyp.ml appear to be set by the parameters of the Strategy command. The description of that command made sense. "expansion weight" suggests something like w1*item1 + w2*item2 + ... +wn*itemn, which is more complex than what the command description says. The command description uses "level", which seems a better fit than "weight" as well as "expand", which I used in the updated messages.

  strategy_level:
    [ [ IDENT "expand" -> { Conv_oracle.Expand }
      | IDENT "opaque" -> { Conv_oracle.Opaque }
      | n=integer -> { Conv_oracle.Level n }
      | IDENT "transparent" -> { Conv_oracle.transparent } ] ]
  ;

image

@herbelin herbelin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I added a few other things made by About. Still not exhaustive though. For instance, it also mentions the dependencies in section variables.

Comment thread doc/sphinx/proof-engine/vernacular-commands.rst Outdated
Comment thread doc/sphinx/proof-engine/vernacular-commands.rst Outdated
@herbelin

Copy link
Copy Markdown
Member

The command description uses "level", seems a better fit than "weight" as well as "expand", which I used in the updated messages.

Then fine to me.

@herbelin herbelin self-assigned this Oct 28, 2024
@jfehrle
jfehrle force-pushed the about_transparency branch from 0b738e9 to 8b4af72 Compare October 28, 2024 16:58
@jfehrle

jfehrle commented Oct 28, 2024

Copy link
Copy Markdown
Member Author

Added your changes. Should be ready to merge. Thanks!

@herbelin

Copy link
Copy Markdown
Member

OK, will merge by tomorrow or Wednesday if there are no more comments.

@herbelin herbelin removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Oct 30, 2024
@herbelin

Copy link
Copy Markdown
Member

@coqbot run full ci

1 similar comment
@herbelin

Copy link
Copy Markdown
Member

@coqbot run full ci

@jfehrle

jfehrle commented Nov 3, 2024

Copy link
Copy Markdown
Member Author

OK, will merge by tomorrow or Wednesday if there are no more comments.

Please?

@herbelin

herbelin commented Nov 4, 2024

Copy link
Copy Markdown
Member

@coqbot merge now

@coqbot-app
coqbot-app Bot merged commit 9538650 into rocq-prover:master Nov 4, 2024
@proux01 proux01 modified the milestones: 8.20.1, 9.0+rc1 Nov 4, 2024
@proux01

proux01 commented Nov 4, 2024

Copy link
Copy Markdown
Contributor

Let's not backport a change in output (there is still a little risk that someone relies on it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: documentation Additions or improvement to documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants