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

Add plugin zip import command #558

Conversation

JoshuaBehrens
Copy link
Contributor

@JoshuaBehrens JoshuaBehrens commented Feb 20, 2020

1. Why is this change necessary?

There is still no really good way to version community store plugins.

  1. sw:store:download is not available in shopware 6 yet
  2. packages.friendsofshopware.com is not officially supported

Now you can at least version a downloaded plugin file and simply do:
bin/console plugin:zip-import SwagBundle.zip && bin/console plugin:install --activate SwagBundle

2. What does this change do, exactly?

Add a new command to import plugin zip files. Add optional parameter to disable file deletion on import via CLI.

3. Describe each step to reproduce the issue or behaviour.

  1. Have a previously downloaded plugin zip file
  2. Open /admin
  3. Open plugin management
  4. Upload zip file
  5. Install plugin
  6. Activate plugin
  7. Be annoyed that it is not possible via CLI in an easy way to do the same

4. Checklist

  • I have written tests and verified that they fail without my change
  • I have squashed any insignificant commits
  • I have written or adjusted the documentation according to my changes
  • This change has comments for package types, values, functions, and non-obvious lines of code
  • I have read the contribution requirements and fulfil them.

@pantrtxp
Copy link
Contributor

Hi @JoshuaBehrens

Thanks again for your contribution.
Please add tests, documentation and watch for deprecations before we proceed with importing your PR

Sunny regards
Pantrtxp

@JoshuaBehrens
Copy link
Contributor Author

Wait? What deprecations? Am I already using these? Isn't this a task for a reviewer that there is such a conflict?

Where are any commands documented that I have to do as well?

Ok I will look for a way to test this.

@JoshuaBehrens
Copy link
Contributor Author

I added docs and tests. I did not test the command itself as there are no templates how to do that yet but I test my changed behaviour.

@shopwareBot
Copy link

Hello,

thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/NEXT-7340

Please use this issue to track the state of your pull request.

Copy link
Contributor

@pantrtxp pantrtxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JoshuaBehrens
could you please have a look at these requests?

src/Core/Framework/DependencyInjection/plugin.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@pantrtxp pantrtxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JoshuaBehrens
could you please have a look at these requests?

Copy link
Contributor

@pantrtxp pantrtxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JoshuaBehrens
could you please have a look at these requests?

Copy link
Contributor

@pantrtxp pantrtxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JoshuaBehrens
could you please have a look at these requests?

Copy link
Contributor

@pantrtxp pantrtxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JoshuaBehrens
could you please have a look at these requests?

@pantrtxp
Copy link
Contributor

Sorry for this comment fuck up but github had some issues posting my comment

@JoshuaBehrens
Copy link
Contributor Author

It is ok :) and if you did it on purpose it is ok for me as well. I also post some nonsense here as well 😅

@pantrtxp
Copy link
Contributor

pantrtxp commented Mar 9, 2020

@JoshuaBehrens would you accept these changes?
this is the last step to getting this merged :)

@JoshuaBehrens
Copy link
Contributor Author

I'll get on it ASAP 🚀 :shipit:

@pantrtxp
Copy link
Contributor

@JoshuaBehrens push again 🙈

@JoshuaBehrens
Copy link
Contributor Author

Yes I am aware of this being open. Gimme time for the weekend. That looks good for it.

homer hedge

@pantrtxp
Copy link
Contributor

Sadly here are some failures as well.
Could you run the phpunit tests locally (after rebasing) and check out the failing ones?
Tests: 44, Assertions: 365, Failures: 18.

We are currently fixing the travis CI so you can rely on this again
Sorry for the inconvenience :(

@JoshuaBehrens
Copy link
Contributor Author

After a rebase lots of tests fail as a refresh_token table is missing although I ran all the migrations :/

@pantrtxp
Copy link
Contributor

pantrtxp commented May 5, 2020

Hi @JoshuaBehrens
This PR is still incomplete.
Will you please update this PR?

Sunny regards
Pantrtxp

@JoshuaBehrens
Copy link
Contributor Author

I can retry testing it but the last time I tried my tests were not working because of a missing google client dependency.

I will also change my doc updates for the new layout.

@ssltg
Copy link
Contributor

ssltg commented May 27, 2020

Hi @JoshuaBehrens,
can you take a look again, your test suite works again I've heard through the grapevine ;-)

@JoshuaBehrens
Copy link
Contributor Author

JoshuaBehrens commented May 27, 2020

Spooky

But yes, I'll do

@mitelg
Copy link
Member

mitelg commented May 28, 2020

*once your branch is rebased 😉

@JoshuaBehrens JoshuaBehrens force-pushed the feature/plugin-zip-import-command branch from 046efd6 to 1a375fd Compare June 5, 2020 11:28
@JoshuaBehrens
Copy link
Contributor Author

Hey @mitelg rebase is out. I hope I included all expectations we had during this conversation

@JoshuaBehrens
Copy link
Contributor Author

Tests look good :)

@shopwareBot
Copy link

Hello,

thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/NEXT-9223

Please use this issue to track the state of your pull request.

@mitelg
Copy link
Member

mitelg commented Jun 17, 2020

thanks for your contribution @JoshuaBehrens 👍 🎉 💙

@mitelg mitelg added Accepted and removed Scheduled labels Jun 17, 2020
@mitelg
Copy link
Member

mitelg commented Jun 17, 2020

merged with d464970

@JoshuaBehrens
Copy link
Contributor Author

Thanks :) You're welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants