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

Deactivation of style checks #1079

Open
treiher opened this issue Jun 20, 2022 · 4 comments
Open

Deactivation of style checks #1079

treiher opened this issue Jun 20, 2022 · 4 comments
Labels
architectural decision Discussion of design decision specification Related to specification package (e.g., specification parsing)

Comments

@treiher
Copy link
Collaborator

treiher commented Jun 20, 2022

Context and Problem Statement

In some cases, the style checks cannot be fulfilled or it is not intended to fulfill all style checks. In such cases, it should be possible to disable certain style checks.

Use Cases

UC1: Generated specifications

In particular, the line length checks cannot be guaranteed in all cases.

UC2: Protyping

During prototyping, the style checks could be annoying.

Considered Options

O1 Global deactivation of style checks (e.g., by CLI option)

No differentiation between different files possible (e.g., manually written specification vs. generated specification)

O2 Local deactivation using comments

Specific style checks can be disabled using comments on a file level, block level or line level similar to the messages control of pylint.

All checks can be disabled using style: disable = all.

Example
         Untagged_Value_data_trap_Value_specific_trap_Untagged_Length : Prelude::Asn_Length
            then Untagged_Value_data_trap_Value_specific_trap_Untagged_Value
               with Size => Untagged_Value_data_trap_Value_specific_trap_Untagged_Length * 8;
         -- style: disable = line-too-long
         Untagged_Value_data_get_next_request_Value_variable_bindings_Untagged_Value : RFC1157_SNMP::Asn_Raw_SEQUENCE_OF_VarBind
            then null;
         -- style: enable = line-too-long

+ Flexible

Decision Outcome

O2 (initially, only support style: tag at the beginning of the file)

@treiher treiher added specification Related to specification package (e.g., specification parsing) architectural decision Discussion of design decision labels Jun 20, 2022
@treiher treiher added this to To do in RecordFlux 0.6 via automation Jun 20, 2022
@kanigsson
Copy link
Collaborator

It seems to me that the CLI switch fits best for prototyping, and the comments are best for generated code. Ideally we would have both solutions then. A way to make the comments work for prototyping would be to have a single comment at the top of the file that disables all style checking.

@rami3l
Copy link
Contributor

rami3l commented Jun 21, 2022

I have found the "pragma style" proposed in the snippet a bit misleading, since pylint also supports disabling all or some lints just for one line.

What about:

         Untagged_Value_data_trap_Value_specific_trap_Untagged_Length : Prelude::Asn_Length
            then Untagged_Value_data_trap_Value_specific_trap_Untagged_Value
               with Size => Untagged_Value_data_trap_Value_specific_trap_Untagged_Length * 8;
         -- style: disable-next = line-too-long
         Untagged_Value_data_get_next_request_Value_variable_bindings_Untagged_Value : RFC1157_SNMP::Asn_Raw_SEQUENCE_OF_VarBind
            then null;

See also: # noqa in flake8

@treiher
Copy link
Collaborator Author

treiher commented Jun 21, 2022

@rami3l The example just shows one example for disabling style checks on a block level. The description of O2 explicitly mentions that the checks could also be disabled for single lines or the whole file, similarly as it is done with pylint. The noqa of flake8 only allows to suppress checks on single lines and thus is much less powerful.

@rami3l
Copy link
Contributor

rami3l commented Jun 21, 2022

@treiher
Thanks for your clarification.

I had carefully read the description for O2 and the pylint link you posted before posting the comment, and I'm aware of the requirement of toggling checks on the block level.

I just wanted to point out that:

  • For the example you posted, the "pragma" style seems a bit too verbose;
  • From a codegen's point of view, adding something like # noqa at the end of each "long" line while printing the file seems like a good starting point.

@treiher treiher self-assigned this Jun 21, 2022
@treiher treiher moved this from To do to Implementation in RecordFlux 0.6 Jun 21, 2022
treiher added a commit that referenced this issue Jun 21, 2022
treiher added a commit that referenced this issue Jun 21, 2022
@treiher treiher removed this from Implementation in RecordFlux 0.6 Jun 21, 2022
@treiher treiher added this to To do in RecordFlux Future via automation Jun 21, 2022
treiher added a commit that referenced this issue Jun 23, 2022
treiher added a commit that referenced this issue Jun 23, 2022
treiher added a commit that referenced this issue Jun 24, 2022
treiher added a commit that referenced this issue Jun 24, 2022
@treiher treiher removed their assignment Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architectural decision Discussion of design decision specification Related to specification package (e.g., specification parsing)
Projects
No open projects
Development

No branches or pull requests

3 participants