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

we need to prefer package over group settings #2687

Merged
merged 1 commit into from Aug 30, 2017

Conversation

Projects
None yet
2 participants
@matthid
Member

matthid commented Aug 29, 2017

we need to prefer package over group settings, fixes #2682

Note: I didn't fully understand the root cause but (especially why a particular comparison contained "None"), but it seems to be obvious that we need to prefer package over group settings (and therefore have them in front of the "+"). @forki could you please quickly review if that makes sense?

This might have lead to incorrect comparisons of restrictions (but I don't get why I saw in the debugger a comparsion of "generate_load_scripts = Some true" with "generate_load_scripts = None"

From my understanding it should never be "None" no matter the order of the arguments?!

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 29, 2017

Member

Ah OK I found the reason:
GenerateLoadScripts is not part of the (+) operator:
https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/Versioning/Requirements.fs#L773-L787

Should it?

Member

matthid commented Aug 29, 2017

Ah OK I found the reason:
GenerateLoadScripts is not part of the (+) operator:
https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/Versioning/Requirements.fs#L773-L787

Should it?

@0x53A 0x53A referenced this pull request Aug 29, 2017

Open

Diagnostics: warn if a with-operation updates all values #603

4 of 5 tasks complete
@0x53A

This comment has been minimized.

Show comment
Hide comment
@0x53A

0x53A Aug 29, 2017

Contributor

My guess: GenerateLoadScripts was added after the record was created, and the adder forgot to update the operator? See commit f616c8b

I created fsharp/fslang-suggestions#603 ;)

Contributor

0x53A commented Aug 29, 2017

My guess: GenerateLoadScripts was added after the record was created, and the adder forgot to update the operator? See commit f616c8b

I created fsharp/fslang-suggestions#603 ;)

@matthid matthid merged commit c26f20d into master Aug 30, 2017

2 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment