PDGGK commented on PR #118:
URL: https://github.com/apache/iotdb-extras/pull/118#issuecomment-5180368128

   Implemented all three outcomes from the list thread, plus the licence point. 
Pushed as `8409aaa`.
   
   **1. Minimum version.** `grafanaDependency` stays at `>=12.3.0`, and the 
README now states the floor, why the externals model makes an untested floor 
meaningless, and how CI holds it. `CHANGELOG.md` records the compatibility 
break.
   
   The smoke test is `minimum-grafana-version` in `grafana-plugin.yml`. It 
parses the floor out of `plugin.json` rather than hardcoding a version, boots 
that Grafana release alongside IoTDB, and asserts two different things: that 
the plugin's own config fields render — which is where an unresolved 
`@grafana/*` external actually surfaces — and that a table-model query returns 
rows through the backend and the native client. So raising or lowering the 
declared floor changes what is tested, with nothing else to keep in step. If 
the version cannot be parsed the step exits non-zero rather than falling back 
to a default, so it cannot silently drift into testing the wrong release.
   
   I checked it fails for the right reasons rather than only that it passes: 
with Grafana stopped it fails; with IoTDB stopped only the query assertion 
fails while the two frontend ones stay green; with `dist/` removed it fails. 
The middle one is the case I most wanted to rule out, since a two-part smoke 
test is usually undone by one half masking the other.
   
   **2. AGENT files.** Removed, and `.config/AGENTS/` added to `.gitignore` so 
a scaffold regeneration cannot bring them back.
   
   **3. LICENCE / RAT.** The blanket `**/grafana-plugin/**` exclusion is 
replaced by a targeted list. Rather than guess at it I lifted the exclusion and 
measured: 68,238 unapproved files, all of them `node_modules/` and the 
downloaded `node/`. Excluding generated output, binaries and files that 
physically cannot carry a header brought that to 8, and adding the standard 
header to those eight hand-written files brought it to 0. `.config/` stays 
excluded with the reason inline — the scaffold regenerates it and marks it 
do-not-edit, so a header added there is lost on the next run.
   
   One thing worth flagging rather than deciding on my own:
   
   **On the Maven path.** Confirmed: before this PR neither Grafana profile was 
invoked by any CI job or by the Jenkinsfile, so nothing was building this 
module through Maven. The new job runs `-Pwith-grafana-plugin`, so that path is 
now exercised on every change to the plugin directory.
   
   One thing I found while checking, since it bears on whether to go further: 
the module is reachable from two opt-in profiles, `with-grafana-plugin` and 
`with-grafana` — and `with-grafana` is part of the build line the top-level 
README recommends 
(`-Pwith-all-connectors,with-examples,with-springboot,with-grafana`), so anyone 
following the README is already building it locally. It is still in neither the 
default build nor `with-all-connectors`. If you would like it in a default 
profile as well I am happy to do that, but it changes build time for every 
connector, so it seemed worth asking rather than folding into this PR.
   
   One observation I am deliberately keeping out of this PR since it is 
pre-existing and untouched here: `backend-compile.sh` runs `go get -u` and `go 
mod tidy` at build time, so a build can rewrite `go.mod`. Both of my builds 
bumped `magefile/mage` from v1.15.0 to v1.17.2; I reverted it before committing 
each time. I will open that separately once this lands.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to