There's a lot of configuration keys that rightfully should not be configurable, and there are several plugin names and configuration values that could use better names.

Right now is probably the best time to affect these things as once its released and in use, we'll have a much harder time swapping them around without concerning ourselves with BC.

Which, btw, we should *probably* include some mechanism for versioning drupalci.yml files before we cant.

Should probably make that just like docker-compose.yml files do, which is a key at the top of the file:
version: 1

  • consider renaming simpletest plugin to run_tests to make it clear that its coupled to that script.
  • Remove all of the 'skip-linting' config options. - we either need a standard config option on buildtaskbase that allows us to disable a plugin, *or* we need standardize on 'just comment it out/delete it from your build'. Individual plugins do not need individual solutions to 'not running'.
  • die-on-nonzero/die-on-fail/fail-should-terminate/sniff-fails-test/lint-fails-test -> make all of these options consistent. Pretty much the same concept throughout. If a plugin fails, should it stop the build, or keep processing.
  • executable-path in container_composer : this should just be a variable inside of the plugin, not a configuration option.
  • start-directory / installed-paths in phpcs plugin - also things that nobody should need to change, and that we should be able to derive from the build.
  • always-use-drupalci-yml : #2953938: Remove update_build's always-use-drupalci-yml config

Comments

Mixologic created an issue. See original summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes

  • e19a920 committed on 2956081-api-cleanup 
    Issue #2956081: renames simpletest occurances to either development for...

  • 172636b committed on 2956081-api-cleanup 
    Issue #2956081: halt-on-fail is shorter
  • 9d6d9b9 committed on 2956081-api-cleanup
    Issue #2956081: standardizes on 'halt-on-failure'

  • e784cec committed on 2956081-api-cleanup 
    Issue #2956081: removes executable-path as a config option in the...
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

simpletest and simpletest_d7 plugins are now run_tests and run_tests_d7 plugins, with an empty proxy plugin that extends the run-tests plugins as a BC layer for six or so months after deployment.

the simpletest.yml and simpletestlegacy7.yml jobs have been renamed after assessment targets, namely 'development' and 'legacydevelopment'

The die-on-nonzero/etc list are all now standardized on 'halt-on-fail' (except 'die-on-fail', which is actually a run-tests.sh option)

The container_composer now has a property on the Composer class for the executable. This isn't something we really need to change all that often, so not even really sure it needs to be a variable, but it wont hurt so there it is.

So, that leaves the start-directory and installed-paths for phpcs.

Those are a little more complicated, so Im going to open up an child issue for those. #2956394: Cleanup config options in phpcs plugin

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Noticed this in a couple places...

-      simpletest.standard:
+      run_tests.standard:
         types: 'Simpletest,PHPUnit-Unit,PHPUnit-Kernel,PHPUnit-Functional'
-      simpletest.js:
+      run_tests.standard:
         concurrency: 1
         types: 'PHPUnit-FunctionalJavascript'

Otherwise, looks great. run_tests is much better than simpletest.

  • 8fee461 committed on 2956081-api-cleanup 
    Issue #2956081: oops global edits

  • 1af7dce committed on 2956081-api-cleanup 
    Issue #2956081: ooremoves unused trait that snuck in somewhere else
  • 2b9e298 committed on 2956081-api-cleanup
    Issue #2956081: eliminates the skip linting config option for csslint
  • 555f84e committed on 2956081-api-cleanup
    Issue #2956081: eliminates the skip linting config option for csslint

  • 1e1e16e committed on 2956081-api-cleanup 
    Issue #2956081: refactors the source/config directory functions on...

  • 4db9df3 committed on 2956081-api-cleanup 
    Issue #2956081:  Many more cleanups, removing installed-paths for...
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Active » Fixed

Well heyo.. all the tests here pass. probabaly going to wait on this to deploy, however. Big things afoot tomorrow that makes me not want to take any additional risk until the smoke clears

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.