Skip to content

Conversation

@TylerVigario
Copy link
Contributor

No description provided.

@thellimist
Copy link
Contributor

thellimist commented Nov 4, 2017

Could you run linter? (Fixing CI)

Thanks

@IcyApril
Copy link
Contributor

IcyApril commented Nov 4, 2017

+1 @thellimist

@MeekLogic - Thanks for contributing. Could you also make sure you add an associated test for this method in tests/Endpoints/ZonesTest.php ?

Thanks!

@TylerVigario
Copy link
Contributor Author

TylerVigario commented Nov 6, 2017

Just seen this, will add the appropriate test shortly.

EDIT: Might be stupid but I think I made a proper test but I don't really know how to test.

@IcyApril
Copy link
Contributor

IcyApril commented Nov 6, 2017

OK, thanks @MeekLogic. All that's left is to fix the build - could you please run make fix and then commit the changes?

Alternatively if you cd into the project directory, you can probably get away with just running something like:

php ./vendor/bin/php-cs-fixer fix --config=.php_cs

@TylerVigario
Copy link
Contributor Author

TylerVigario commented Nov 7, 2017

@IcyApril Thank you very much for pointing me in the right direction.

Threw the PHP CS Fixer PHAR in the directory and ran the command and it worked like magic!

Side note: What environment (OS) are you using? Or is this like some Docker thing lol. I need to get with this CI/CD stuff.

@thellimist
Copy link
Contributor

We're using Travis CI. https://github.com/cloudflare/cloudflare-php/blob/master/.travis.yml
For style control we're using phpcs-fixer and phpmd.
And a Makefile https://github.com/cloudflare/cloudflare-php/blob/master/Makefile

Hope this answers your question.

Thanks

Renamed toggleDevelopmentMode to changeDevelopmentMode to be uniform
@TylerVigario
Copy link
Contributor Author

Thanks @thellimist but I could have gathered that information. What I could not ascertain is what is required of someone to issue those commands within their local environment.

Copy link
Contributor

@IcyApril IcyApril left a comment

Choose a reason for hiding this comment

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

string $since = "-10080", string $until = "0"

Should these be integers or strings?

@IcyApril
Copy link
Contributor

IcyApril commented Nov 8, 2017

@MeekLogic Try switching to assertObjectHasAttribute as is the case in testListZones:

        $this->assertObjectHasAttribute('result', $result);
        $this->assertObjectHasAttribute('result_info', $result);
        $this->assertEquals("023e105f4ecef8ad9ca31a8372d0c353", $result->result[0]->id);
        $this->assertEquals(1, $result->result_info->page);

@TylerVigario
Copy link
Contributor Author

TylerVigario commented Nov 8, 2017

@IcyApril I believe strings, in the curl example he sends timestamp values

This value can be a negative integer representing the number of minutes in the past relative to time the request is made, or can be an absolute timestamp that conforms to RFC 3339.

https://api.cloudflare.com/#zone-analytics-dashboard

@IcyApril
Copy link
Contributor

IcyApril commented Nov 8, 2017

Yeah, that should be fine. We should just make sure the tests are appropriate.

@TylerVigario
Copy link
Contributor Author

TylerVigario commented Nov 8, 2017

Installing PHPMD within VSCode to fix those pesky errors.

Edit: It doesn't like the boolean parameters for some reason

The method changeDevelopmentMode has a boolean flag argument $enable, which is a certain sign of a Single Responsibility Principle violation.

@TylerVigario TylerVigario changed the title Added toggleDevelopmentMode to Zones Added changeDevelopmentMode & getAnalyticsDashboard to Zones Nov 8, 2017
@IcyApril IcyApril merged commit 3453e44 into cloudflare:master Nov 8, 2017
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

Successfully merging this pull request may close these issues.

3 participants