-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Move final few dependencies to Composer #63
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linting failed (3 errors, 6 warnings).
9 notices occurred in your codebase, but none on files/lines included in this PR.
Any guidance on how y'all would like to deal with the linting errors would be appreciated! I've seen the namespace error on other projects, but I'm not sure how to resolve it. I can see us ignoring the |
@nathanielks I think most of those are ones I'd add |
"humanmade/wordpress-pecl-memcached-object-cache": "2.0.0" | ||
"humanmade/wordpress-pecl-memcached-object-cache": "2.0.0", | ||
"humanmade/batcache": "1.3.0", | ||
"stuttter/ludicrousdb": "4.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you maybe run a diff on 4.2.0 just to make sure we didn't have any local modifications to ludicrous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like perhaps a copy went wrong at some point? Those green files are files that are in nested directories, but the plugin itself beyond those are the same. The only file we directly reference is ludicrousdb/includes/class-ludicrousdb.php
Ok good stuff. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the release for aws-ses you pushed in https://github.com/humanmade/aws-ses-wp-mail/tree/1.0.0 includes the AWS SDK, which we don't want to do, as that is dead code that won't be used. Typically on other projects, we've have something like a packagist
branch where we'd remove that, and tag the releases from that.
@joehoyle I wonder if it's worth just updating it to be a composer-only package and ship that as 1.1? Having to maintain a separate branch would be annoying. |
Yeah I’m good with that. We did that with s3-uploads (and provide a downloadable built zip for people that don’t want to instal via composer) in our case I don’t think we need to do that on ses as it’s not super widely used
…--
Joe Hoyle
CTO
Human Made
On Wed, Sep 11 2019 at 14:02, Daniel Bachhuber < ***@***.*** > wrote:
@joehoyle ( https://github.com/joehoyle ) I wonder if it's worth just
updating it to be a composer-only package and ship that as 1.1? Having to
maintain a separate branch would be annoying.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (
#63?email_source=notifications&email_token=AABHPE5735J6ZEJ6HSEK2ETQJEXCDA5CNFSM4ISEZS32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6PLUXQ#issuecomment-530496094
) , or mute the thread (
https://github.com/notifications/unsubscribe-auth/AABHPE4KHWXA7G4V7ISZKBDQJEXCDANCNFSM4ISEZS3Q
).
|
I opened a PR at humanmade/aws-ses-wp-mail#41 to remove the AWS SDK from the AWS-SES plugin. |
I've cut 1.1.0 for aws-ses-wp-mail and updated the composer dep here. |
Title says it all! Depends on humanmade/altis-core#54