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

Testing latest branch #22

Open
marcbria opened this issue Apr 10, 2019 · 2 comments
Open

Testing latest branch #22

marcbria opened this issue Apr 10, 2019 · 2 comments

Comments

@marcbria
Copy link
Contributor

marcbria commented Apr 10, 2019

Hi @lucasdiedrich

First, thanks a lot for your time and work and sorry if I'm slow with the feedback or trying to understand how all it works. Beginnings are hard, so please, be patient because we are close to got the "machine greased" ;-)

Today I got time to work on this. Let me say that PKP team is really interested in this work and they like somebody outside their core is taking the lead on this. They usually have their hands full, but we can get full support if we need it.

So... let's work:

Following your request in #14 I ignore the "tags version" (although is something to talk in future, don't we?) and I started testing the "php7-test" branch.

To make it run I just made the following:

$ git clone https://github.com/lucasdiedrich/ojs.git
$ cd ojs
$ git fetch
$ git checkout php7-test
$ docker-compose up 

Good news are that now you get a full functional and pre-installed OJS 3.1.1-4 in your 8085 port.
Bad news we have a few inconsistencies to be addressed (OJS 3.1.1.4, php5, variables ...).

  • docker-compose.yml refers to "latest":
    • Need to be "php7-test"... or we will test the wrong image.
    • Why not refering the local dockerfile instead of dockerhub's?
  • Consistent variables between .env and docker-compose:
    • SERVERNAME: ${PROJECT_DOMAIN:-pkp.ojs.localhost}
    • "${HTTP_PORT:-8085}:80"
    • "${HTTPS_PORT:-8445}:443"

PR with those changes here: #23

About the rest...

I have doubts about some points. I will open a different issue for each case (with a PR) so we can discuss them separately and you can accept refuse the PR, but in short:

  • Commenting volumes by default is good , thanks! but I have an improvement proposal:
    • What about a different folder structure to distinguish between "volumes", "config files" and "scripts" (let's talk in the new issue).
    • What about naming volumes? Otherwise, when you have some ojs, it's a mess.
  • Do we need an "ENVIRONMENT" section if we have an ENV file?
  • I'm unsure if OJS_CLI_INSTALL need to be 1 or 0 by default.
  • Minor README.md fixings.

If is ok for you I will close former discussions to keep the issue tracker clean.

Thanks again for your time,
m.

@marcbria
Copy link
Contributor Author

marcbria commented Apr 10, 2019

The patched docker-compose (#23) with "php7-test" image works fine, except for a little detail.

First time you go to the journal, you get the install page (that is not supposed to be there because of OJS_CLI_INSTALL is set to 1... but the funny part is this info is ignored (ie: "admin" pwd is the default one). Unsure about why it happens. :-(

As said, I tested in deep but apart of this detail, you get a full functional OJS 3.1.2 with php 7.2.14 that works like a charm.

Great work @lucasdiedrich, thanks!

PS to @Potomac54: file is included in php7-test image.

@lucasdiedrich
Copy link
Owner

s again for your tim

Thanks for your patch, and yes please, open the issues so we can discuss about the new fixes and improvements.

About the OJS_CLI_INSTALL not working over 3.1.2 it should be because one or more environment variables are wrong, we can manually call de ojs-cli-install manually to check that.

About the files directories, inside this project will only have one folder, which is the files used by the container. The problem that we're having is that the docker-compose its in the root of the project, and we by mistake can think that we need that folder. That's why once i sugested to move the docker-compose inside another folder called examples.

When i said to forget the tags version it was because we needed an example version so i can upgrade the tags version now. Which i can maybe do today, at least de 3.1.1.4 version.

Thanks for all you help @marcbria

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

No branches or pull requests

2 participants