-
Notifications
You must be signed in to change notification settings - Fork 1
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
ENH Use repositories from supported-modules #24
Conversation
1918d01
to
6fcd800
Compare
@@ -119,7 +118,7 @@ abstract protected function fetchDataFromApi(string $path, string $postBody = '' | |||
|
|||
protected function getMethod(string $postBody): string | |||
{ | |||
return $postBody ? Consts::METHOD_POST : Consts::METHOD_GET; | |||
return $postBody ? 'post' : 'get'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated refactoring
@@ -29,7 +28,7 @@ protected function fetchDataFromApi(string $path, string $postBody = ''): string | |||
$url = $apiConfig->deriveUrl($path, $offset); | |||
Logger::singleton()->log("REST {$ucMethod} {$url}"); | |||
curl_setopt($ch, CURLOPT_URL, $url); | |||
if ($method == Consts::METHOD_POST) { | |||
if (strtolower($method) == 'post') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated refactoring
6fcd800
to
f1c63ac
Compare
@@ -2,7 +2,6 @@ | |||
|
|||
namespace { | |||
|
|||
use App\DataFetcher\Models\ApiData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated refactoring, unused import
128513e
to
fe8acf6
Compare
fe8acf6
to
a7bc573
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue #3
I also did some unrelated refactoring to get rid of the pointless get/post consts so that I could delete Consts.php
I've done work to not show unsupported modules in ci builds table e.g. ckan build will show for CMS 4, but not for CMS 5
I have not done the same for merge-ups as there's now shared logic between gha-merge-up and rhino, so the CMS 5 branches for ckan will still show. I personally don't think this is too big of a deal, though if we want to fix this we should do this as a separate card as gha-merge-up would need to be updated as well and this card is only for rhino.