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

refactor: remove dead code in OSL decoder #7886

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

anmonteiro
Copy link
Collaborator

We're already in the else branch, we never return `Modes modes

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@anmonteiro anmonteiro force-pushed the anmonteiro/rm-dead-code branch from 73a8297 to 71fa894 Compare June 5, 2023 06:56
Copy link
Collaborator

@jchavarri jchavarri left a comment

Choose a reason for hiding this comment

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

I would have left the code mostly as is, except the block inside decode_osl, .e.g:

        try_
          (Mode_conf.Lib.Set.decode >>| fun modes -> `Modes modes)
          (fun exn ->
            try_
              ( Mode_conf.Lib.Set.decode_osl ~stanza_loc project >>| fun modes ->
-               if dune_version >= expected_version then `Modes modes
-               else `Upgrade )
+              `Upgrade)
              (fun _ -> raise exn))
        >>| function
        | `Modes modes -> modes
        | `Upgrade ->
          Syntax.Error.since stanza_loc Stanza.syntax expected_version
            ~what:"Ordered set language for modes"

src/dune_rules/dune_file.ml Show resolved Hide resolved
src/dune_rules/dune_file.ml Show resolved Hide resolved
Copy link
Collaborator

@jchavarri jchavarri left a comment

Choose a reason for hiding this comment

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

sorry for the distraction

@rgrinberg
Copy link
Member

Do we have a test that covers this?

@anmonteiro
Copy link
Collaborator Author

Yeah, check the tests in #6611 which also tests the error behavior.

@anmonteiro anmonteiro merged commit bbe3d1f into ocaml:main Jun 5, 2023
@anmonteiro anmonteiro deleted the anmonteiro/rm-dead-code branch June 5, 2023 20:35
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.

3 participants