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

Remove --hard flag #1567

Merged
merged 3 commits into from Apr 5, 2016

Conversation

Projects
None yet
3 participants
@forki
Member

forki commented Apr 4, 2016

Background

--hard: Replaces package references within project files 
          even if they are not yet adhering to the Paket's conventions 
          (and hence considered manually managed).

Sample

in default mode paket 2.0 will not change references to libraries if they are not already controlled by paket. A common case is "file/ enw F# project" creates a fsproj file that already references FSharp.Core. If you then add FSharp.Core as nuget package, then Paket will not replace the reference unless you specify --hard parameter.

Suggestion

I suggest we remove the --hard flag in paket v3 and thread all calls as if --hard would have been set.

  • removed flag from all commands
  • fixed tests
  • update docs

We will not create a --soft flag to restore old behaviour, but if you really need to keep that existing reference you can manually add a

<paket>false</paket>

forki added some commits Apr 4, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 4, 2016

Member

This is obviously breaking, but it was a big source for confusion.

I'm only suggesting this for v3 so I'm relaxed about it like this guy:

pizza

Member

forki commented Apr 4, 2016

This is obviously breaking, but it was a big source for confusion.

I'm only suggesting this for v3 so I'm relaxed about it like this guy:

pizza

@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Apr 4, 2016

Contributor

This is good, it's what I think people expect anyway as the default.

Contributor

isaacabraham commented Apr 4, 2016

This is good, it's what I think people expect anyway as the default.

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Apr 4, 2016

Contributor

It fine to remove it in my opinion. But what would happen if I try to use it ? Will I gen an error or it just will be ignored ? I prefer not to have an error, this way I don't need to update my scripts.

Contributor

tsibelman commented Apr 4, 2016

It fine to remove it in my opinion. But what would happen if I try to use it ? Will I gen an error or it just will be ignored ? I prefer not to have an error, this way I don't need to update my scripts.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 4, 2016

Member

With this pr we would throw an error: unknown parameter --hard
On Apr 4, 2016 8:28 PM, "tsibelman" notifications@github.com wrote:

It fine to remove it in my opinion. But what would happen if I try to use
it ? Will I gen an error or just it will be ignored ? I prefer not to have
an error this way I don't need to update my scripts.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1567 (comment)

Member

forki commented Apr 4, 2016

With this pr we would throw an error: unknown parameter --hard
On Apr 4, 2016 8:28 PM, "tsibelman" notifications@github.com wrote:

It fine to remove it in my opinion. But what would happen if I try to use
it ? Will I gen an error or just it will be ignored ? I prefer not to have
an error this way I don't need to update my scripts.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1567 (comment)

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Apr 4, 2016

Contributor

Can this be a warning instead ?

Contributor

tsibelman commented Apr 4, 2016

Can this be a warning instead ?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 4, 2016

Member

Since this is a major release change I would prefer not to take that debt.
On Apr 4, 2016 8:43 PM, "tsibelman" notifications@github.com wrote:

Can this be a warning instead ?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1567 (comment)

Member

forki commented Apr 4, 2016

Since this is a major release change I would prefer not to take that debt.
On Apr 4, 2016 8:43 PM, "tsibelman" notifications@github.com wrote:

Can this be a warning instead ?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1567 (comment)

@isaacabraham

This comment has been minimized.

Show comment
Hide comment
@isaacabraham

isaacabraham Apr 5, 2016

Contributor

Agree with @forki - this a major release with a breaking change that should be documented as part of the release.

Contributor

isaacabraham commented Apr 5, 2016

Agree with @forki - this a major release with a breaking change that should be documented as part of the release.

@forki forki merged commit 3e43fbd into v3 Apr 5, 2016

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@forki forki deleted the reallyhard branch Apr 5, 2016

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