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

NEW Use a single-source-of-truth to determine version mapping #107

Conversation

GuySartorelli
Copy link
Member

$versions = array_keys($config['versions']);

foreach ($versions as $version) {
$modulesUrl = "https://raw.githubusercontent.com/silverstripe/supported-modules/$version/modules.json";
Copy link
Member Author

Choose a reason for hiding this comment

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

This requires the supported-modules PRs to be merged to work correctly - but if you want to test this locally in the mean time you can replace that one URL with this code:

switch ($version) {
    case '5':
        $modulesUrl = 'https://raw.githubusercontent.com/silverstripe/supported-modules/e9c9d44a6833bda035d6a3182effd167ab499549/modules.json';
        break;
    case '4':
        $modulesUrl = 'https://raw.githubusercontent.com/silverstripe/supported-modules/daa19e8c279f599088ba12b2a91b9a51cfbc5eb2/modules.json';
        break;
    case '3':
        $modulesUrl = 'https://raw.githubusercontent.com/silverstripe/supported-modules/30ceab854f964f4488f2834b80fc4d56f464e9c2/modules.json';
        break;
}

Comment on lines +7 to +9
bin/docs checkout -v \
&& DOCTUM_COMPOSER_AUTOLOAD_FILE='./vendor/autoload.php' ./vendor/bin/doctum.php --version \
&& DOCTUM_COMPOSER_AUTOLOAD_FILE='./vendor/autoload.php' ./vendor/bin/doctum.php --ignore-parse-errors --force -v update conf/doctum.php
Copy link
Member Author

Choose a reason for hiding this comment

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

We shouldn't try the doctum code if we failed to build out the version map.

@GuySartorelli GuySartorelli force-pushed the pulls/master/supported-modules branch 2 times, most recently from 038d868 to e127ab5 Compare April 20, 2023 05:29
"phpunit/phpunit": "^7",
"phpunit/phpunit": "^8.5",
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed to use $this->expectWarning() in a test.

'silverstripe/graphql/src/Extensions/IntrospectionProvider.php',
0
);
public function provideGetRepoUrl()
Copy link
Member Author

Choose a reason for hiding this comment

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

These are all the same tests that were happening before (except the graphql 3 one which was silly - that's not a valid mapping) plus one for CMS 5 and 3 just so we have one for each of the three mapped versions.

Comment on lines 13 to 25
public static function setUpBeforeClass(): void
{
$config = Config::getConfig();
$filePath = Config::configPath($config['paths']['versionmap']);
if (file_exists($filePath)) {
self::$fileContent = file_get_contents($filePath);
unlink($filePath);
}
file_put_contents($filePath, json_encode([
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to make sure there's some mapping available to test against. But if you're running tests locally you may already have a map there so we make sure to keep that and restore it after the test.

Comment on lines +17 to +28
/**
* Regex patterns for github repositories that should be ignored.
* These are repositories that don't have relevant API.
*/
private array $ignoreModulePatterns = [
'/^silverstripe\/developer-docs$/',
'/^silverstripe\/installer$/',
'/^silverstripe-themes\/simple$/',
'/^silverstripe\/vendor-plugin$/',
'/^silverstripe\/recipe-.*/',
'/-theme$/',
];
Copy link
Member Author

Choose a reason for hiding this comment

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

These are repositories which either have no PHP code at all, or their PHP code isn't really relevant.
The second part of that might be fairly subjective though.... feel free to recommend changes here if you think of any changes that are appropriate.

@GuySartorelli GuySartorelli changed the base branch from master to supported-modules April 24, 2023 01:01
@emteknetnz emteknetnz merged commit b1113f5 into silverstripe:supported-modules Apr 24, 2023
@emteknetnz emteknetnz deleted the pulls/master/supported-modules branch April 24, 2023 01:03
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.

None yet

2 participants