wp-cli / dist-archive-command

Create a distribution .zip or .tar.gz based on a plugin or theme's .distignore file
https://developer.wordpress.org/cli/commands/dist-archive/
MIT License
47 stars 23 forks source link

Move command over to new v2 structure #27

Closed wojsmol closed 6 years ago

wojsmol commented 6 years ago

fixes https://github.com/wp-cli/dist-archive-command/issues/26

wojsmol commented 6 years ago

Here we need Circle CI for PR's.

wojsmol commented 6 years ago

Any progress on enabling Circle CI for PR's?

schlessera commented 6 years ago

@wojsmol As far as I can tell, CircleCI should be active here...

wojsmol commented 6 years ago

@schlessera Yes, but not for PR's - see https://circleci.com/docs/1.0/fork-pr-builds/ - https://github.com/wp-cli/dist-archive-command/pull/27/commits/edf497d17b93296cff31211b34d8158e3d8e93e5 was not start a build

wojsmol commented 6 years ago

PR is ready forb review. For now bash script is used to setup tests.

wojsmol commented 6 years ago

database setup moved to Circle CI config file. PR is ready for review.

schlessera commented 6 years ago

Hey @felicianotech,

Are you able to help us out here? I'd like to use MYSQL_HOST in the CircleCI config to avoid hardcoding any environment-specific information into the scripts we're using (they are used with both CIs and local environments).

My current investigation lead me to believe that the problem is that --skip-name-resolve is hardcoded for the MySQL docker image, so the grants for root@% don't work for root@localhost.

Is there something I can do within the CircleCI config to get around this issue?

FelicianoTech commented 6 years ago

This is the build error I see:

Access denied for user 'wp_cli_test'@'127.0.0.1'

At minimum I would add the environment variables:

MYSQL_USER: wp_cli_test
MYSQL_PASSWORD: password1
MYSQL_HOST: 127.0.0.1

under the MySQL image as well. The way the image is designed to work, it will automatically create a user wp_cli_test with the password password and the hostname 127.0.0.1 for you then. Then you just need to connect with those creds.

FelicianoTech commented 6 years ago

For example, that config section could be like this:

    docker:
      - image: circleci/php:7.1-cli
          environment:
            MYSQL_DB: wp_cli_test_scaffold
            MYSQL_USER: wp_cli_test
            MYSQL_PASSWORD: password1
            MYSQL_HOST: 127.0.0.1
      - image: circleci/mysql:5.7
          environment:
            #MYSQL_DB: wp_cli_test_scaffold # Having this here would create this database name for you in the DB, though it will be empty (no tables)
            MYSQL_USER: wp_cli_test
            MYSQL_PASSWORD: password1
            MYSQL_HOST: 127.0.0.1
    steps:
      - checkout
schlessera commented 6 years ago

@felicianotech Seems like we still have the same problem with these changes. The DB does not seem to map % to localhost...

wojsmol commented 6 years ago

@felicianotech composer prepare-tests execute this script.

FelicianoTech commented 6 years ago

I'm going to fork this repo and setup my own CI for my fork to see if i can help debug this.

I should needed any private environment variables for anything right? Since I'm not deploying?

FelicianoTech commented 6 years ago

Okay I got it to work. @wojsmol, I'll open a PR to your fork shortly.

Update:

Okay I opened a PR to fix this: https://github.com/wojsmol/dist-archive-command/pull/2/files

Once that is merged there, this PR will begin to pass here.

schlessera commented 6 years ago

@felicianotech Thank you very much for the help!