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 ready to install packages to each PR #28409

Merged
merged 5 commits into from
Mar 25, 2020

Conversation

HLeithner
Copy link
Member

@HLeithner HLeithner commented Mar 20, 2020

There was a discussion somewhere to have ready to install and upgrade packages for each PR. This PR address this request.

Summary of Changes

Add a new drone step to build packages and notify github when the packages are ready.
You will find an additional entry in the "check" section of the PR with a link to the packages.

image

@zero-24
Copy link
Contributor

zero-24 commented Mar 20, 2020

Is there a reason that this is only for 4.0-dev can this later also be done for the 3.x branches?

@HLeithner
Copy link
Member Author

Is there a reason that this is only for 4.0-dev can this later also be done for the 3.x branches?

Yes should be possible for j3 too, I only started with j4 in the first place because it seams people still have problems to use composer and npm.

@zero-24
Copy link
Contributor

zero-24 commented Mar 20, 2020

sure would be great to get it working for 3.x too than 👍 Awesome feature to be sure!

@dgrammatiko
Copy link
Contributor

sure would be great to get it working for 3.x too

The reason that this is needed for the J4 branch more than the 3.x is down to the composer and npm steps that many non devs cannot execute. J3 doesn't have those limitations but still having an installable per PR is a helping hand. Also once this is in for both branches the patch tester can be archived

@HLeithner
Copy link
Member Author

Also once this is in for both branches the patch tester can be archived

Actually Patchtester has a new maintainer @roland-d so we hopefully see progress on it. Beside this the patchtester solves a different problem, it's much easier to test multiple PRs and revert them. that's not possible if you install a complete package.

@mbabker
Copy link
Contributor

mbabker commented Mar 20, 2020

So what is the actual benefit to this added build step on every pull request then, since as you’re pointing out it’s not really addressing anything for an end user? Joomla doesn’t support rollback, and barely supports updates if you’re not on a stable tag, so these packages seem to just add more steps to a tester’s workflow because once it is installed into a site then the site has to be destroyed to revert the changes. About the only benefit is for testing the updater, which being realistic, less than 5% of all pull requests require testing that functionality.

This to me is just another facade to say “it’s easy to test Joomla changes” by hiding all of the technical knowledge necessary to actually work with patches or git or Composer or NPM. The project is better off not using these tools over continuing to invest resources it doesn’t have into hiding the fact these tools are necessities for code contributions to this and a growing number of other repositories.

@brianteeman
Copy link
Contributor

Also once this is in for both branches the patch tester can be archived

This isnt a replacement for patchtester in any way

@HLeithner
Copy link
Member Author

So what is the actual benefit to this added build step on every pull request then, since as you’re pointing out it’s not really addressing anything for an end user? Joomla doesn’t support rollback, and barely supports updates if you’re not on a stable tag, so these packages seem to just add more steps to a tester’s workflow because once it is installed into a site then the site has to be destroyed to revert the changes. About the only benefit is for testing the updater, which being realistic, less than 5% of all pull requests require testing that functionality.

Actually we have many tester and part time testers and reports who are not able to use composer and npm because they have simple a website and don't do any development. So we simply open the possibility to test to a bigger group of people, especially them we lost when starting to use composer and npm.

This to me is just another facade to say “it’s easy to test Joomla changes” by hiding all of the technical knowledge necessary to actually work with patches or git or Composer or NPM. The project is better off not using these tools over continuing to invest resources it doesn’t have into hiding the fact these tools are necessities for code contributions to this and a growing number of other repositories.

Of course because a tester doesn't need to know anything about the technical infrastructure to test an issue. And the repository is actually only a new branch in our existing docker-images repo, yes one more that's true but we also try to get rid of other unmaintained infrastructure.

@brianteeman
Copy link
Contributor

I doubt that your target user is going to do a full download and full install

@HLeithner
Copy link
Member Author

I doubt that your target user is going to do a full download and full install

yes and of course doing updates to test them

@ChristineWk
Copy link

HLeithner wrote: > Actually we have many tester and part time testers and reports who are not able to use composer and npm because they have simple a website and don't do any development. So we simply open the possibility to test to a bigger group of people, especially them we lost when starting to use composer and npm.

... as me & other Users :-)

@mbabker
Copy link
Contributor

mbabker commented Mar 20, 2020

So before this is deployed then I would highly suggest that a workflow is devised for supporting rollback/revert. Because for the level of tester you are targeting here, you are going to get someone who just installs Joomla fresh using the package on the pull request, or if you’re lucky someone who updates a test site using the package on the pull request, but you have left them stranded and unable to further test using that site.

There is a reason I keep saying that people who are testing pull requests should be able to work with git at a minimum. It’s not because I think only the technically privileged should do pull request review and testing (although sometimes I wish that were the case with the number of times I have code reviewed a pull request that was supposedly tested successfully but could have never worked because they introduced different kinds of fatal errors). It’s because from a software workflow perspective, the right tools for the job are the tools necessary to create the pull request in the first place (git, Composer, NPM). Joomla is just not designed as a piece of software that you can do in place testing and revert the changes when you’re done testing. Patch tester, no matter how friendly you make the UI or how much you overengineer it under the hood, will always suffer the same flaw, not because the component is poorly written but because the software it is written for is not intended for that use case.

@HLeithner
Copy link
Member Author

HLeithner commented Mar 20, 2020

So before this is deployed then I would highly suggest that a workflow is devised for supporting rollback/revert. Because for the level of tester you are targeting here, you are going to get someone who just installs Joomla fresh using the package on the pull request, or if you’re lucky someone who updates a test site using the package on the pull request, but you have left them stranded and unable to further test using that site.

You know this is not possible with our current database model

There is a reason I keep saying that people who are testing pull requests should be able to work with git at a minimum. It’s not because I think only the technically privileged should do pull request review and testing (although sometimes I wish that were the case with the number of times I have code reviewed a pull request that was supposedly tested successfully but could have never worked because they introduced different kinds of fatal errors). It’s because from a software workflow perspective, the right tools for the job are the tools necessary to create the pull request in the first place (git, Composer, NPM). Joomla is just not designed as a piece of software that you can do in place testing and revert the changes when you’re done testing. Patch tester, no matter how friendly you make the UI or how much you overengineer it under the hood, will always suffer the same flaw, not because the component is poorly written but because the software it is written for is not intended for that use case.

So you say every software tester out there is able to compile the software they are testing? Actually most of the tester doesn't even be able to get the source of the software they are testing. Beside that nightly builds have the exact same problem but they are ok?

If you think it's needed to create a information at the download site that this files are only for tests and should never be installed on a live site and can't be upgraded and 10 other warnings I'm fine with it. But to say "hey you tested joomla the last 15 years and now you are to dumb to use our (partly hard to use and install) dev chain" sounds not as a good solution to me. And we get complaints about this on a regular base.

And yes our user base are not drupal users, till the marketing team finds it out what our main target group is, I assume that our users are "normal" people creating websites (basically I think most of our users are integrators building websites for others and have a hard time creating overrides) and want to help to make joomla better.

Beside that I completely understand your approach require a minimum knowledge our workflow but for testing an ui element in the backend you don't need to know git.

@mbabker
Copy link
Contributor

mbabker commented Mar 20, 2020

You’re coming to this with the assumption that everyone who clicks the download button on .org should be able to test a pull request from GitHub on a Joomla site. I don’t think that should be the case.

Yes, for some people, it is going to be easier to either install Joomla fresh from a package on a pull request or update a testing site using a package on a pull request then trash the entire installation when they’re done. But to me, that is an inefficient use of people’s time.

WordPress doesn’t have all this extra crap. Can’t use a patch file? Too bad. And they are the king of assuming their users are so stupid that PHP and MySQL are considered profane words instead of trusted software.

So, disregard my feedback. The project generally does anyway.

@mbabker
Copy link
Contributor

mbabker commented Mar 20, 2020

My stance hasn’t changed over the years. If the tools are too complex for your target user, then the tools need to be changed to match the target user expectations. You’re saying testers are too scared or not savvy enough to use Composer and NPM? To me the solution is to not use those tools, not waste your time, and an entire team’s time, coming up with software solutions that are half assed and don’t really solve anything practical. If the tools are deemed essential to the software’s workflow, then the software should be helping people use those tools.

@HLeithner
Copy link
Member Author

You’re coming to this with the assumption that everyone who clicks the download button on .org should be able to test a pull request from GitHub on a Joomla site. I don’t think that should be the case.

Yes, for some people, it is going to be easier to either install Joomla fresh from a package on a pull request or update a testing site using a package on a pull request then trash the entire installation when they’re done. But to me, that is an inefficient use of people’s time.

WordPress doesn’t have all this extra crap. Can’t use a patch file? Too bad. And they are the king of assuming their users are so stupid that PHP and MySQL are considered profane words instead of trusted software.

So, disregard my feedback. The project generally does anyway.

Basically I didn't created this container because it was so much fun, I created because there are requests for this or similar stuff since years and if we can get one tester more to be active we have 10% more test capacity. And no I'm not disregard your feedback.

My stance hasn’t changed over the years. If the tools are too complex for your target user, then the tools need to be changed to match the target user expectations. You’re saying testers are too scared or not savvy enough to use Composer and NPM? To me the solution is to not use those tools, not waste your time, and an entire team’s time, coming up with software solutions that are half assed and don’t really solve anything practical. If the tools are deemed essential to the software’s workflow, then the software should be helping people use those tools.

Once again a tester doesn't need to understand the developer tool chain in my opinion. So we should give the tester the possibility to contribute without being a developer.

If this tool is complete useless we can remove it, it's 10 lines of code.

@mbabker
Copy link
Contributor

mbabker commented Mar 20, 2020

Define a tester. If they are anyone who is able to download a package from .org and install Joomla on a web server, then you’re 100% correct in that they should be totally oblivious to Joomla’s software development workflow. If that is not the definition of a tester, then understanding what the project’s development lead considers a tester would be beneficial in guiding future work.

@HLeithner
Copy link
Member Author

Define a tester. If they are anyone who is able to download a package from .org and install Joomla on a web server, then you’re 100% correct in that they should be totally oblivious to Joomla’s software development workflow. If that is not the definition of a tester, then understanding what the project’s development lead considers a tester would be beneficial in guiding future work.

You are right we talk about a completely different group of testers, as already mentioned if you only need to test a UI element in the Backend you don't need any knowledge except how to install joomla. If you want to test the API you still don't need to know our tool chain because you maybe in the python universe and only want to test the webservice. If you want to test a feature in joomla that touches model you should have strong knowledge how joomla works and what this could be affected (if the pr creator didn't mention everything in the instructions).

So yes these builds are for a limited group of persons and should not replace any other testing workflow.

@conconnl
Copy link
Member

conconnl commented Mar 20, 2020

I have read the whole discussion and HLeithner is correct to look at a solution for the Joomla Administrators who are not developers. (Like me)

I can’t use the strange NPM & Composer things, they will certainly not work on the Hosting Packages supplied by different companies.

To make it work on my local device I need a complete manual, which defines every step and support when I get an error.
Most people in user groups want to help and they can if they can just follow a joomla installer or have a always working (or supported if not) patch tester.
I would rather follow the install process to even be able to test that, with an update package to test a fix after confirming the issue in a new install then using the patch tester.

No, @HLeithner should not think this is for just the people who wants a joomla website.

No, @mbabker should not think that joomla administrators are developers or server engineers who understand NPM, Composer or have local systems to use.

@roland-d
Copy link
Contributor

@mbabker

My stance hasn’t changed over the years. If the tools are too complex for your target user, then the tools need to be changed to match the target user expectations. You’re saying testers are too scared or not savvy enough to use Composer and NPM? To me the solution is to not use those tools, not waste your time, and an entire team’s time, coming up with software solutions that are half assed and don’t really solve anything practical.

Are you sure your stance hasn't changed? You did maintain the patchtester for years and now you are saying they are a waste of time.

WordPress doesn’t have all this extra crap.

Do they have a larger userbase of technically priviliged coders to help testing?

Define a tester.

The way I see it, we have different levels of testers. The people in like usergroups who can test the easy ones like GUI elements or behaviour. Then there are the people who are familiar with the Joomla codebase and can make a better call whether or not the code is good enough. Finally there is the group of people that can program but don't use Joomla.

I think only the technically privileged should do pull request review and testing (although sometimes I wish that were the case with the number of times I have code reviewed a pull request that was supposedly tested successfully but could have never worked because they introduced different kinds of fatal errors).

If we can only allow full-fledged Joomla developers to test, then we might as well close up shop because we don't even have enough of those to write all the patches.

then understanding what the project’s development lead considers a tester would be beneficial in guiding future work.

Any guidance for the future is welcome :)

@N6REJ
Copy link
Contributor

N6REJ commented Mar 20, 2020

@chmst
Copy link
Contributor

chmst commented Mar 21, 2020

The way I see it, we have different levels of testers. The people in like usergroups who can test the easy ones like GUI elements or behaviour. Then there are the people who are familiar with the Joomla codebase and can make a better call whether or not the code is good enough. Finally there is the group of people that can program but don't use Joomla.

fully agree. There are countless PRs for small UI elements, and some people are out there who would like to test. But do not understand local environments or forking on github.
Why not involve them in testing if there is a possibility?
I gave up wasting my time for UI tests and running npm.

If we can only allow full-fledged Joomla developers to test, then we might as well close up shop because we don't even have enough of those to write all the patches.

Who is in this group? Sorry to say that but when new contributors make or test PR, the "gurus" should encourage and mentor them instead of demotivate. Who today can test only a border around a table, maybe he or she can test tomorrow the bigger PRs.

Where I agree is that reviewing code can do only by high level developers.

@ChristineWk
Copy link

As you can install the zip patch and than (excluding PRs with database changes) just hint the "reinstall" button it is not required to do a per patch install.

but the Re-Install button provides only the previous status of 4.0.0-dev - no sql update files.
I think so.

@zero-24
Copy link
Contributor

zero-24 commented Mar 22, 2020

This is why i said (excluding PRs with database changes). There is no revert to version X feature yet in joomla ;)

@HLeithner
Copy link
Member Author

HLeithner commented Mar 23, 2020

Since there is still the question if this pr is useful I added 2 things to the docker image.

  1. Add an update manifest for easy update testing
  2. Add a landing page with information and warnings for easier use

Based on point 1. the update test of tobias can use this ready to install packages for update tests.

If this container works I will port it to staging and 3.10

@brianteeman
Copy link
Contributor

how to test?

@HLeithner
Copy link
Member Author

how to test?
When packaging is ready the link will be an entry "download" in the checks section. For the current build:

https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/28409/downloads/30563

For the last (unsigned) build the url is:
https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/28409/downloads/30560

@brianteeman
Copy link
Contributor

The first of those links gives a 404

As for the text on the second page and the menu links. Where can that be edited as its neither english nor logical

@wilsonge
Copy link
Contributor

As for the text on the second page and the menu links. Where can that be edited as its neither english nor logical

https://github.com/joomla-projects/docker-images/blob/packager/templates/index.html

@brianteeman
Copy link
Contributor

Thanks @wilsonge I will give it a go tomorrow

@HLeithner
Copy link
Member Author

First link is ready in the meantime, the text is copied and (pretty sure not good) modified from https://developer.joomla.org/nightly-builds.html

@brianteeman
Copy link
Contributor

@HLeithner Am I correct in assuming that the target user of this is the less technical user? If I am then the text needs completely rewriting as its targeted at developers. Builds, commits, pull requests are meaningless to the target user

@HLeithner
Copy link
Member Author

@brianteeman I would be happy if you write a better text and make a PR against https://github.com/joomla-projects/docker-images/tree/packager

@brianteeman
Copy link
Contributor

started a pr

@zero-24
Copy link
Contributor

zero-24 commented Mar 25, 2020

I'm merging this now in for 4.0 thanks @HLeithner for this great work to make the life of our testers and automated tests even more easy where possible. 👍

@zero-24 zero-24 merged commit b6913c3 into joomla:4.0-dev Mar 25, 2020
@zero-24 zero-24 added this to the Joomla 4.0 milestone Mar 25, 2020
@ChristineWk
Copy link

Also thanks from me to @HLeithner for your great work!
Even, I didn't understand all technical features :-)

I will install (again) a fully nightly. Should I use this one? https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/28409/downloads/30563/

@zero-24
Copy link
Contributor

zero-24 commented Mar 25, 2020

You can but in the best case you install the nightly from here: https://developer.joomla.org/nightly-builds.html

@alikon
Copy link
Contributor

alikon commented Mar 26, 2020

silly question
is this feature working for the pr that have been submitted/changed after this has been merged ?
so no download package+pr for the oldest ?

@alikon
Copy link
Contributor

alikon commented Mar 26, 2020

as an example this pr #27005 doesn't have the download link

Screenshot from 2020-03-26 19-02-46

@dgrammatiko
Copy link
Contributor

@alikon just retrigger drone 😉

@richard67
Copy link
Member

Just wanted to say the same ;-)

@alikon
Copy link
Contributor

alikon commented Mar 26, 2020

i didn't have that super power 🐤

plus shouldn't we need to trigger a batch task for trigger drone for all the oldest pr's ?

@wilsonge
Copy link
Contributor

People would need to update their branch to the latest 4.0 to have this in their drone configs before you reran. I don't think it's worth the effort. just let it be phased in.

@alikon
Copy link
Contributor

alikon commented Mar 26, 2020

this doesn't help the peoples for which this pr was intended for

@HLeithner HLeithner deleted the feature/packager branch March 29, 2020 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.