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

Allow filtered updates of packages matching a regex #1178

Merged
merged 7 commits into from Oct 30, 2015

Conversation

Projects
None yet
2 participants
@mavnn
Contributor

mavnn commented Oct 30, 2015

Especially useful for situations like "update all packages that start with my organisations prefix"

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 30, 2015

Member

do you have a small sample for this? Then I could add a regression test

Member

forki commented Oct 30, 2015

do you have a small sample for this? Then I could add a regression test

@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

Of course.

Given dependency file:

source https://nuget.org/api/v2

nuget Castle.Windsor                           
nuget NUnit                             
nuget Microsoft.AspNet.WebApi.SelfHost

Then:

paket update nuget Microsoft.* --filter should update AspNet

paket update nuget [MN].* --filter will update NUnit and AspNet but leave Castle.Windsor untouched.

Without the --filter flag, package names are treated as exact matches. This is potentially important as . is a special character in regexs.

Contributor

mavnn commented Oct 30, 2015

Of course.

Given dependency file:

source https://nuget.org/api/v2

nuget Castle.Windsor                           
nuget NUnit                             
nuget Microsoft.AspNet.WebApi.SelfHost

Then:

paket update nuget Microsoft.* --filter should update AspNet

paket update nuget [MN].* --filter will update NUnit and AspNet but leave Castle.Windsor untouched.

Without the --filter flag, package names are treated as exact matches. This is potentially important as . is a special character in regexs.

Show outdated Hide outdated src/Paket.Core/PackageResolver.fs
@@ -189,6 +189,7 @@ type Resolved = {
type UpdateMode =
| UpdatePackage of GroupName * PackageName

This comment has been minimized.

@forki

forki Oct 30, 2015

Member

can we remove that case and express it as UpdateFiltered of GroupName * PackageFilter?

@forki

forki Oct 30, 2015

Member

can we remove that case and express it as UpdateFiltered of GroupName * PackageFilter?

This comment has been minimized.

@mavnn

mavnn Oct 30, 2015

Contributor

As in, escape .s and treat it as a filter? I think it would work. Hope your regression tests are good up to date though! :D

@mavnn

mavnn Oct 30, 2015

Contributor

As in, escape .s and treat it as a filter? I think it would work. Hope your regression tests are good up to date though! :D

This comment has been minimized.

@mavnn

mavnn Oct 30, 2015

Contributor

Give me a few minutes and I'll see if all the tests pass.

@mavnn

mavnn Oct 30, 2015

Contributor

Give me a few minutes and I'll see if all the tests pass.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki
Member

forki commented Oct 30, 2015

image

@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

My code obviously rules :)

Contributor

mavnn commented Oct 30, 2015

My code obviously rules :)

@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

Mind pushing that test to my master branch? I'll add it to the pull request.

Contributor

mavnn commented Oct 30, 2015

Mind pushing that test to my master branch? I'll add it to the pull request.

@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

Ah, ok - I'll grab it.

Contributor

mavnn commented Oct 30, 2015

Ah, ok - I'll grab it.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki
Member

forki commented Oct 30, 2015

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 30, 2015

Member

you need to run build.cmd runintegrationtests since we don't execute that target during standard build

Member

forki commented Oct 30, 2015

you need to run build.cmd runintegrationtests since we don't execute that target during standard build

@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

Should having working unit and integration tests now

Contributor

mavnn commented Oct 30, 2015

Should having working unit and integration tests now

|> ignore)
|> shouldFail
[<Test>]
let ``SelectiveUpdate does not update any package when package does not exist``() =

This comment has been minimized.

@mavnn

mavnn Oct 30, 2015

Contributor

I removed this test as it's expectation didn't seem meaningful anymore. With pattern based updates, a pattern that didn't match any packages should be a no-op, not an exception.

The update package command still checks that the specific named package exists before calling UpdateFiltered

@mavnn

mavnn Oct 30, 2015

Contributor

I removed this test as it's expectation didn't seem meaningful anymore. With pattern based updates, a pattern that didn't match any packages should be a no-op, not an exception.

The update package command still checks that the specific named package exists before calling UpdateFiltered

@@ -736,11 +720,11 @@ let ``SelectiveUpdate updates package that conflicts with transitive dependency
nuget Ninject.Extensions.Interception
nuget Ninject.Extensions.Logging""")
let packageName = PackageName "Ninject.Extensions.Logging"
let packageFilter = PackageFilter "Ninject\\.Extensions\\.Logging"

This comment has been minimized.

@forki

forki Oct 30, 2015

Member

it would be cool if we would have a function which transform PackageNames into PackageFilter

@forki

forki Oct 30, 2015

Member

it would be cool if we would have a function which transform PackageNames into PackageFilter

This comment has been minimized.

@mavnn

mavnn Oct 30, 2015

Contributor

Static members PackageName.toFilter and PackageFilter.ofName?

@mavnn

mavnn Oct 30, 2015

Contributor

Static members PackageName.toFilter and PackageFilter.ofName?

This comment has been minimized.

@forki

forki Oct 30, 2015

Member

yes something like that. then we could keep the tests more stable

@forki

forki Oct 30, 2015

Member

yes something like that. then we could keep the tests more stable

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 30, 2015

Member

Could also please write one or two samples in the docs of the update command?

Member

forki commented Oct 30, 2015

Could also please write one or two samples in the docs of the update command?

@mavnn mavnn merged commit 27f7f2f into fsprojects:master Oct 30, 2015

1 of 2 checks passed

continuous-integration/appveyor Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

@forki crap. Push to wrong remote by mistake. Sorry.

Contributor

mavnn commented Oct 30, 2015

@forki crap. Push to wrong remote by mistake. Sorry.

@mavnn

This comment has been minimized.

Show comment
Hide comment
@mavnn

mavnn Oct 30, 2015

Contributor

/me goes off to change default push repository

Contributor

mavnn commented Oct 30, 2015

/me goes off to change default push repository

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 30, 2015

Member

no worries ;-)

Member

forki commented Oct 30, 2015

no worries ;-)

forki added a commit that referenced this pull request Oct 30, 2015

@pms1969 pms1969 referenced this pull request Jan 12, 2016

Closed

Errors when building #1379

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