Repository navigation
Name each example's engines by package in 51d.json - #110
Merged
Merged
Conversation
…g them for the published packages
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
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.
Problem
Eleven of the examples'
51d.jsonfiles name an engine by a path into this repository's source, such as"../../../deviceDetectionCloud"indevice-detection/fiftyone.devicedetection.cloud/examples/cloud/gettingstarted-console/51d.jsonat fe81e018. Such a file only works beside the source.ci/install-package.ps1lines 43 to 55 rewrites two of them so the Publish tests run them against the packed packages, anddevice-detection/run_examples.mdlines 72 to 92 andip-intelligence/run_examples.mdlines 61 to 81 tell a person running an example against the published packages to edit the rest by hand. This is gap DX-node-20, left under "What remains" in #95, which named the mixed web example's engine by package.PipelineBuilder.buildFromConfigurationrequires eachelementNamefrom the core package (pipeline/fiftyone.pipeline.core/pipelineBuilder.jslines 143 to 150), so a package name, such asfiftyone.devicedetection.cloud/deviceDetectionCloud, resolves both in this repository, where npm workspaces link the packages, and in an application that installed them.What the specification says
device-detection-specification/required-examples.mdlines 20 to 23 at c8128ca4 asks for each example to be run "as someone would run it from a fresh clone", and the examples rule asks that each also builds and runs against the published packages, which a path into the source cannot.What changed
fiftyone.devicedetection.cloud/deviceDetectionCloud,fiftyone.devicedetection.cloud/hardwareProfileCloudEngine,fiftyone.devicedetection.onpremise/deviceDetectionOnPremise,fiftyone.ipintelligence.cloud/ipIntelligenceCloud,fiftyone.ipintelligence.onpremise/ipIntelligenceOnPremiseandfiftyone.pipeline.engines.fiftyone/shareUsage.ci/install-package.ps1no longer rewrites two of them, as the files now work against the installed packages as they are.run_examples.mdfiles say the files work unchanged with the installed packages, in place of the steps to edit them.The helpers that make a relative name absolute, such as
OptionsExtension.updateElementPath, act only on a name that starts with., so they leave these names alone.Tests
ci/tests/exampleConfigurations.test.jsreads every51d.jsonunder the four product folders and checks that each element is named by package and resolves from the core package, asPipelineBuilderrequires it.Both ran in a throwaway workflow that is not part of this draft. The examples' own tests run each of these files in the full build below.
Full build
Run 43569113470 on
tmp/pr-matrix-110, started bytools/pr-matrix.pywithout macOS. All 7Setting 'PullRequestSha'lines give this head, 1f96238, and Configure printed "Left out 2 macOS entries". Each of the six Build and Test jobs, Ubuntu, Ubuntu ARM and Windows on Node.js 22 and 24, passed 1,412 unit tests with 2 skipped and 133 integration tests with 4 skipped, among them the new configuration test and the tests of the examples that read these files, on Windows as well as Linux.macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
What remains
The Publish workflow's test of the published packages runs the two web examples that
ci/install-package.ps1used to rewrite, and now runs them on these files as they are. That is next shown by the Publish dry run of the release head.