search

TreewareEarth / plant

Composer plugin that bothers the consumer of Treeware packages.
MIT License
13 stars 0 forks source link

Treeware Plan(t) release (composer plugin) #2

Closed ostark closed 3 years ago

ostark commented 3 years ago

What is this package about?

I've some free hours to spend next week and would love to prepare a release.
As I don't see the need to overcomplicate things with a GH milestone or discussion boards, instead, I'd love to get your input here:

1) Does the package what we want? (high level) 2) Are the config options for package author (the man in the middle) right? 3) Is the output during composer install/update okay? 4) Who can think of some useful unit tests? (I'd go with pest, but I have no idea for a meaningful test ;-)) 5) What else is needed to make it more useful?

Ping: @jamesmills @philsturgeon @Gummibeer

Maybe @tomdavies & @unlikenesses / @alexpestell?

pxlrbt commented 3 years ago

Is the output during composer install/update okay?

I think the output is okay so far. I'd maybe add some kind of stats for the package as proposed in #1

What else is needed to make it more useful?

I don't know the Composer internals. Can we add a flag to disable the output for CI/CD pipelines?

Gummibeer commented 3 years ago

I don't know the Composer internals. Can we add a flag to disable the output for CI/CD pipelines?

I would at least recycle the --no-interaction flag as this is common and best practice for CI environments and as there's no interactivity it's also useless to output treeware details.

ostark commented 3 years ago

I was about to add a check for the --no-interaction flag, but then I noticed, it should not be an issue, since CI/CD environments should never trigger an update or require event (install only) > https://github.com/TreewareEarth/plant/blob/main/src/Plugin.php#L42

More ideas? @pxlrbt @Gummibeer

Gummibeer commented 3 years ago

Not totally right. In case you want to run your tests with different PHP versions or other dependencies switching versions you have to run an update and/or require command. I also use this flag from time to time in my local environment if I just want to get it done. So even if most users won't see a difference I would still respect that flag.

ostark commented 3 years ago

@Gummibeer okay, sold - https://github.com/TreewareEarth/plant/commit/983b209020862ed357e97401d1715be204b2c3c2#diff-c346b1aeac8259782c74702eadfe1d7a240ee3ecea6020ad2addede93c635cabR80

ostark commented 3 years ago

Maybe @jamesmills can help with the README and proper English?

jamesmills commented 3 years ago

This is fantastic. I'll take a deeper look this afternoon. Thanks!

jamesmills commented 3 years ago

My only comment is to remove the priceGroups option. I don't like the idea of people "forcing" a recommended price/amount of trees. I think the general message and gentle reminder is enough.

jamesmills commented 3 years ago

Maybe @jamesmills can help with the README and proper English?

I actually think the README that is already in place is perfect. If we can just update the example to show the latest refactored code that would be amazing.

jamesmills commented 3 years ago

I can see it's already on packagist so I will install this in one of my packages and give it a test this afternoon.

jamesmills commented 3 years ago

I can see it's already on packagist so I will install this in one of my packages and give it a test this afternoon.

Should I be able to install this with composer require treeware/plant master-dev ?

Gummibeer commented 3 years ago

@jamesmills I believe the exact opposite regarding price groups. They can still change. But without guidance they don't buy at all or the lowest possible. Not because they can't effort more but because they don't get an idea of how many trees would be good. It's the same for GitHub Sponsors - they have tiers to give guidance.

jamesmills commented 3 years ago

@jamesmills I believe the exact opposite regarding price groups. They can still change. But without guidance they don't buy at all or the lowest possible. Not because they can't effort more but because they don't get an idea of how many trees would be good. It's the same for GitHub Sponsors - they have tiers to give guidance.

OK, so maybe we keep it in as optional. So if it's there it will be displayed but default is empty.

jamesmills commented 3 years ago

Tested adding this to jamesmills/laravel-timezone and then installed that package into a new local test Laravel project. Worked well.

Screen Shot 2021-04-28 at 3 34 29 PM
ostark commented 3 years ago

Glad you tested it with Composer 1 ;-)

jamesmills commented 3 years ago

Glad you tested in with Composer 1 ;-)

lol...