Conversation
sakanni
force-pushed
the
fix/dataset-pathspec-versioning-upgrade-maps
branch
from
September 9, 2026 09:41
d54d61a to
84197a5
Compare
sakanni
force-pushed
the
fix/dataset-pathspec-versioning-upgrade-maps
branch
from
September 28, 2026 15:04
84197a5 to
44e1e30
Compare
…taset pathspec Follow-up to #20. That exclusion was path-based and covered one directory layout. Versioning_<digits>.json are versioning upgrade maps holding {"Dataset":{"ToNew":..,"ToOld":..}}, read by BHoM_Engine Versioning_Engine/Query/DatasetToNewPaths.cs. They carry no _t, can never be BH.oM.Data.Library.Dataset documents, and sit at a project root in any repo, so a path-scoped rule cannot reach them. Add ':(exclude,glob,icase)**/Versioning_[0-9]*.json' to both tier-bundle templates. Each element is required and is pinned by an assertion: :(glob) keeps the token a filename match rather than a path-substring match, the ** anchor keeps it depth-agnostic, and the [0-9] gate keeps a genuine dataset named Versioning_Rules.json in scope. Flip the two characterisation assertions #20 left as assert_matches, which its own comment recorded as the intended outcome of a widening. Add five fixtures covering the depth-2 case, a real dataset inside a Versioning_<digits> directory, a real dataset under a Versioning_Engine project, and one path that the base selector never reaches. Refine the structural invariant to permit ** only under :(glob). Defect 3 is a property of git's default matcher, where '**' is not special; :(glob) switches on WM_PATHNAME and there '**/' matches zero or more leading components. The invariant previously rejected the only correct way to write a basename match. It still fails ** without :(glob). Suite 83 to 92 assertions. Fleet measurement over 309 repositories on their default branches: no repository loses a file, because no upgrade map currently sits in a selected path on a default branch. Refs BHoM/internal-tickets#36, BHoM/internal-tickets#43.
The pathspec fixtures named a private repository. This is a public repository, so the name is replaced with Example_Datasets. BHoM_Datasets stays as the real, public anchor for the same case. The fixtures assert a path shape: a directory component supplying the 'datasets' substring plus a Versioning_<digits>.json filename. The repository name is arbitrary, so nothing the suite asserts changes. Verified by running the suite before and after the substitution: 92 of 92 both times. Two comments tied a fixture to one specific live failure. They now state what makes the fixture the shape it is instead.
…xtures This is a public repository and the reference pointed into a private one, so no outside reader could open it. It was also mis-aimed: the ticket it named covers a missing Datasets.txt and a fail-open path, not the decision this comment explains. Nothing is lost. The two lines above the reference already state the reason the shape is excluded rather than taught to the runner. Suite unchanged at 92 of 92.
Collaborator
Author
|
Closing: exclusion now lives centrally in the compliance runner rather than the templates. |
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.
Issues addressed by this PR
Follow-up to #20.
ci-dataset-complianceselects on a substring of the whole path, so it picks up files that were neverBH.oM.Data.Library.Datasetdocuments. #20 excluded the versioning test sets by path. This adds the other shape:Versioning_<digits>.jsonupgrade maps, which hold{"Dataset":{"ToNew":..,"ToOld":..}}, carry no_t, and sit at a project root where the filename is the only signal.:(glob), the**anchor and the[0-9]gate are each required. The assertion file explains each one at the fixture that pins it.This edits the two tier-bundle templates, so it reaches future onboardings only. Nothing propagates a template change to the 59 of 61 onboarded repositories already carrying the old pathspec, so on its own this fixes no repository.
Test files
.github/scripts/tests/test-changed-file-patterns.sh, 83 to 92 assertions, all passing.Control runs, each reverting one element:
*Versioning_*.jsonVersioning_93directory case, and the invariant*/**, no digit gateDatasets/Versioning_Rules.jsonLoss check over 309 repositories, whole trees on default branches: 1397 files selected before, 1317 after, and the new token removes nothing beyond #20.
Changelog
ci-dataset-complianceno longer selectsVersioning_<digits>.jsonversioning upgrade maps, at any depth.Additional comments
The structural invariant is loosened. It rejected any token containing
**, which is right for git's default matcher but wrong under:(glob), where**/matches zero or more leading components. As written it forbade the only correct way to express a basename match. It now keys off the token's magic, so**passes under:(glob)and still fails without it.Two assertions flip from
assert_matchestoassert_not_matches, on upgrade maps inside a*_Datasetsproject folder. #20 recorded them as characterisation rather than approval, and flipping them is the intended outcome of a widening rather than a regression.