omise / omise-prestashop

Omise PrestaShop Plugin
https://docs.opn.ooo/prestashop-plugin
MIT License
4 stars 7 forks source link

Composer omise php #61

Closed jonrandy closed 6 years ago

jonrandy commented 6 years ago

1. Objective

Make it easier to maintain the PrestaShop plugin by pulling in omise-php using composer, instead of just having a 'fixed' copy of a particular version of that library stored in the plugin's repo.

2. Description of change

Completely removed the omise-php library from the repo, added a composer.json file, and modified code so that the path to the now 'composer managed' files is correct.

(I also fixed the tests as they were not runnable with recent versions of PHPUnit)

3. Quality assurance

Checked to make sure the plugin still works, which it does

4. Impact of the change

No impact for users as nothing actually changes. Impact is ease of maintenance for devs

5. Priority of change

Normal

6. Additional notes

Calm down England. It was only Panama, it's not like we won the final!

guzzilar commented 6 years ago

@jonrandy how about the installation process? Right now we guide user to download the repo as a zip file n upload directly into their store.

jonrandy commented 6 years ago

Uploading a zip is AFAIK the recommended process in PrestaShop. I could write a small shell script to help devs easily produce the required zip file?

jonrandy commented 6 years ago

Unless of course we add the plugin to the PrestaShop 'store'

jonrandy commented 6 years ago

This PR doesn't affect the install process as the ZIP files for the user to download that we add on the releases are manually created by us. They aren't the same as the source code zip file

jonrandy commented 6 years ago

So, is this one ok?

jacstn commented 6 years ago

I think we need more info in How to test it..

jonrandy commented 6 years ago

That would depend on how you wanted to set up your own dev environment. Personally I have a dockerized solution with the relevant plugin folder inside my container pointing at the omise folder in my local prestashop dev folder. Then I can run composer install to bring in the libs, then test to see if the plugin works as expected.

Here is my docker_compose.yml if you want to set it up that way.

version: '2'

services:
  mariadb:
    image: 'bitnami/mariadb:latest'
    environment:
      - MARIADB_USER=bn_prestashop
      - MARIADB_DATABASE=bitnami_prestashop
      - ALLOW_EMPTY_PASSWORD=yes
    volumes:
      - 'mariadb_data:/bitnami'
  prestashop:
    image: 'bitnami/prestashop:latest'
    environment:
      - MARIADB_HOST=mariadb
      - MARIADB_PORT_NUMBER=3306
      - PRESTASHOP_HOST=localhost
      - PRESTASHOP_DATABASE_USER=bn_prestashop
      - PRESTASHOP_DATABASE_NAME=bitnami_prestashop
      - ALLOW_EMPTY_PASSWORD=yes
    labels:
      kompose.service.type: nodeport
    ports:
      - '80:80'
      - '443:443'
    volumes:
      - './prestashop_data:/bitnami'
      - '../omise-prestashop/omise:/bitnami/prestashop/modules/omise'
    depends_on:
      - mariadb

volumes:
  mariadb_data:
    driver: local
  prestashop_data:
    driver: local
jonrandy commented 6 years ago

@guzzilar The installation instructions are the same. We have always built the zip, and that's what they install. This is merely a change to make our (developers) lives easier

On Sat, 30 Jun 2018, 22:10 Nuttanon Trapnirundon, notifications@github.com wrote:

@guzzilar requested changes on this pull request.

The changes looks fine. Just need to update README.md at the Installation Instruction to make it conform with the change.

Also, need to make sure that we have a built version after we release for the next version v1.4 (it's ok for now since our document pointed to the released-file. but after we release this one, gotta need to make sure that end-user can install it without running composer install)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/omise/omise-prestashop/pull/61#pullrequestreview-133428473, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcLMsrYVUybo9m0pSZOfbmwNYDBDVatks5uB5T5gaJpZM4U3aOZ .