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

CN: Group comamnd line options #346

Merged
merged 1 commit into from
Jul 14, 2024
Merged

CN: Group comamnd line options #346

merged 1 commit into from
Jul 14, 2024

Conversation

dc-mak
Copy link
Contributor

@dc-mak dc-mak commented Jun 24, 2024

This commit groups the current CN options to make it clearer which are used for what.

This could also be useful in preparation for switching to using subcommands.

@dc-mak
Copy link
Contributor Author

dc-mak commented Jun 24, 2024

@cp526 could you check over this? Not familiar with all of these options.

@dc-mak
Copy link
Contributor Author

dc-mak commented Jun 24, 2024

And @rbanerjee20 too.

@dc-mak dc-mak requested review from cp526 and rbanerjee20 June 24, 2024 12:25
@dc-mak dc-mak added the cn label Jun 24, 2024
@rbanerjee20
Copy link
Contributor

For the CN executable spec flags, the grouping of those 3 flags is correct, but I would prefer if the module was named something else, and possibly with a flag suffix since without this it might be confused with existing modules like Executable_spec. E.g. Runtime_checking_flag, Executable_spec_flag (the latter would match the existing naming convention for CN executable spec modules).

@cp526
Copy link
Collaborator

cp526 commented Jun 24, 2024

Agreed about the flag suffix in the module names.

The flag use_peval is probably specific to CN proof; I don't think the executable-spec tool can produce json (though maybe that would be useful in the future?), so the json flag is also specific to CN proof.

@dc-mak dc-mak force-pushed the cn-group-options branch from 8f78b7a to 8e1da98 Compare June 24, 2024 14:12
@dc-mak
Copy link
Contributor Author

dc-mak commented Jun 25, 2024

Thanks - addressed. Any other comments or good to merge?

This commit groups the current CN options to make it clearer which are
used for what.

This could also be useful in preparation for switching to using
subcommands.
@dc-mak dc-mak force-pushed the cn-group-options branch from 8e1da98 to 465adfc Compare July 14, 2024 08:42
@dc-mak dc-mak merged commit a64e1b6 into master Jul 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants