Conversation
CONTRIBUTING.md and the PR template still described the old single-directory layout, from before regressor.py moved into experiment/methods/ (b5c5c96). back then CI copied the files over via scripts/copy_algorithm_files.sh, so contributors only touched algorithms/. that script is gone but the docs never got updated, and nothing catches the mistake: the build-and-test matrix comes from `ls algorithms/`, so a method added only under experiment/methods/ never gets a job and still goes all green (see #211). - CONTRIBUTING.md documents where files go and why it's split that way - PR template reorganized around the two directories - local_ci.sh now wraps the docker commands instead of the old conda flow - scripts/check_method_layout.py checks the two dirs agree; runs in CI as validate-layout and gates build-and-test - fixes algorithms/eql/metadata.yml (invalid yaml), a missing experiment/methods/xgboost/__init__.py, and an invalid escape sequence in gplearn's regressor.py
every push and PR rebuilds all 27 images, because the gate in build-and-test is hardcoded to should_run=true (5b13029). check-changes already computes what changed, but nothing consumes it. one method submission can cost a lot: #209 went through 16 full 27-image runs, #210 and #212 another 24 between them. pull requests now build only the methods they touch. everything else - pushes to master/dev, the new weekly schedule, manual dispatch - still rebuilds everything, so a method that breaks from upstream drift without anyone touching it still gets caught. that drift is calendar-driven, which is what the schedule is for; during a quiet stretch there are no merges to catch it. - a method rebuilds if either algorithms/<name>/ or experiment/methods/<name>/ changed. the second one matters: a regressor.py edit has to retest the method even though the install is untouched. - changes to shared build inputs (dockerfiles, base_environment, scripts, entry.sh, configure.sh, workflows) still rebuild everything. - build-and-test always runs and always reports for every algorithm, so the check names stay present and can be marked required. only the docker build step is skipped. - dropped always() from build-and-test. with the gate inside the job, a failed check-changes would have left an empty build list, skipped every build and reported green. - check-changes no longer diffs against github.event.before, so a force-push to a CI branch no longer fails the job. also fixes a long-standing bug: changed-experiments used awk field $2 on experiment/methods/<name>/..., which is the literal string "methods", not the method name. it needs $3. nothing consumed that output before, so it never showed up.
#220 was squash-merged, so master carried a commit with the same content as dev's 4629556 but a different hash. that made master and dev diverge and turned the next promote into a phantom conflict in ci-docker.yml. dev already contains everything in master, so this records the merge without taking any content. master is an ancestor of dev again and the next promote is a fast-forward. going forward: squash contributor PRs into dev, but promote dev -> master with a merge or fast-forward, never a squash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
brings dev's selective build change onto master. the docs and layout check
from #220 are already here, so the only content diff is ci-docker.yml.
after this, PRs build only the methods they touch. pushes to master/dev, the
weekly schedule and manual dispatch still rebuild everything, so a method
that breaks from upstream drift without anyone touching it still gets caught.