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

Bump lodash to v4 #241

Merged
merged 1 commit into from
May 20, 2016
Merged

Bump lodash to v4 #241

merged 1 commit into from
May 20, 2016

Conversation

samhashemi
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.739% when pulling f87e51b on samhashemi:bump-deps into 987cfbe on AmpersandJS:master.

@samhashemi
Copy link
Contributor Author

@wraithgar @latentflip @cdaringe @pgilad

I've taken a first swing at updating AmpersandJS to lodash v4. Model, View, and Events were updated to v4 in previous pull requests, so I've made 10 additional PRs to update the rest of the project.

A few of the packages depend on other ampersand packages that need to be updated, which I've noted below.

#241
AmpersandJS/ampersand-app#8
AmpersandJS/ampersand-router#52
AmpersandJS/ampersand-class-extend#9
AmpersandJS/ampersand-sync#76
AmpersandJS/ampersand-collection-lodash-mixin#11

Depends on ampersand-sync, updated above:
AmpersandJS/ampersand-collection-rest-mixin#41

Depends on ampersand-class-extend, updated above:
AmpersandJS/ampersand-collection#80
AmpersandJS/ampersand-collection-view#44
AmpersandJS/ampersand-filtered-subcollection#31

If there's anything I can do to help merge these quickly, let me know. They should all be fairly straightforward.

@pgilad
Copy link
Member

pgilad commented May 8, 2016

Verified that all zuul tests run via #242

@pgilad pgilad mentioned this pull request May 8, 2016
@samhashemi
Copy link
Contributor Author

@pgilad thanks for looking through these. is there another person on the ampersand team who can give a second thumbs up?

@pgilad
Copy link
Member

pgilad commented May 11, 2016

ping @AmpersandJS/core-team

@cdaringe
Copy link
Member

+1

1 similar comment
@lukekarrys
Copy link
Contributor

+1

@samhashemi
Copy link
Contributor Author

@cdaringe @lukekarrys thanks. could one of you look through the related commits above? these changes are much more impactful if they are done across the ampersand project -- it makes it possible to transition entirely to lodash v4 instead of requiring two different versions. This can be a huge difference in number of bytes.

@samhashemi
Copy link
Contributor Author

samhashemi commented May 20, 2016

@cdaringe @lukekarrys @pgilad i'm getting the sinking feeling this project isn't being actively maintained.

is there a clear person i should work with to get these merged and pushed to NPM? is the best option forking all the libraries?

@cdaringe
Copy link
Member

hey @samhashemi, sorry to give that impression :/ my experience has told me that we've had the implicit practice of letting the package maintainer take charge of merge/publish. im not sure who does -state, but i'm sure it was just a case of "too-busy." thanks for following up. we'll get it taken care of.

@cdaringe cdaringe merged commit c2b5b58 into AmpersandJS:master May 20, 2016
@cdaringe
Copy link
Member

@pgilad let me know if you want me to publish

@samhashemi
Copy link
Contributor Author

@cdaringe thanks! is there a list of who the maintainer is for each ampersand project? i'd like to get the 10 PRs above merged in parallel.

@latentflip
Copy link
Contributor

@cdaringe I think at one point I was, but I'm taking a back seat from things right now, so if you have publish bits go ahead and do what you think's right :)

@cdaringe
Copy link
Member

cdaringe commented May 20, 2016

@samhashemi, it should be listed on the packages (like here)
@latentflip i'll add ya as such ^^ for this package (even in presence of the backseat comment for now, just to remove ambiguity). if we need to swap out or elect a new lead for this pkg, let's get a new thread goin!

i'll publish this bad boy :)

@cdaringe
Copy link
Member

5.0.2 is up!

@latentflip
Copy link
Contributor

Okay, I definitely shouldn't be listed as I'll be of no use, so I opened this: #243

@pgilad
Copy link
Member

pgilad commented May 20, 2016

Publish, and publish all related repositories as well... I didn't have publish rights to all of them so decided not to get stuck in the middle of the upgrade. I do admit I use ampersand less these days (react + redux mainly) but am happy to contribute when I have time

@samhashemi samhashemi deleted the bump-deps branch May 24, 2016 15:00
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.

6 participants