Skip to content

Add paging support to Rootly- incidents datastream #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
noorulkhan-n wants to merge 4 commits into
base: main
Choose a base branch
from
+22 −10
Open

Add paging support to Rootly- incidents datastream #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e3a405
Add paging support to Rootly incidents datasource
Jun 16, 2026
fd772ea
removed unrecognized module from metadata.json
Jun 16, 2026
f7f5059
version increment
Jun 18, 2026
2d3eb12
adding tags
Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions plugins/Rootly/v1/dataStreams/incidents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,28 @@
{
"name": "incidents",
Comment thread
noorulkhan-n marked this conversation as resolved.
"tags": ["Incidents"],
"config": {
"httpMethod": "get",
"paging": {
"mode": "offset",
"pageSize": {
"realm": "queryArg",
"path": "page[size]",
"value": "200"
},
"offset": {
"rowCountIn": {
"realm": "payloadArraySize",
"path": "data"
},
"mode": "page",
"base": "1"
},
"out": {
"realm": "queryArg",
"path": "page[number]"
}
},
"expandInnerObjects": true,
"endpointPath": "incidents",
"getArgs": [
Expand All @@ -12,14 +33,6 @@
{
"key": "filter[{{timestampField}}][lte]",
"value": "{{{timeframe.end}}}"
},
{
"key": "page[number]",
"value": "1"
},
{
"key": "page[size]",
"value": "100"
}
],
"postRequestScript": "incidents.js",
Expand Down
3 changes: 1 addition & 2 deletions plugins/Rootly/v1/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "rootly",
"displayName": "Rootly",
"version": "1.0.2",
"version": "1.0.3",
"author": {
"name": "@noorulkhan-n",
"type": "community"
Expand All @@ -20,13 +20,12 @@
{
"value": "Bearer {{apiKey}}",
"key": "Authorization"
}
],
"queryArgs": [],
"modules": [],
"authMode": "none",
Comment thread
claude[bot] marked this conversation as resolved.
"baseUrl": "https://api.rootly.com/v1/"
}

Check notice on line 28 in plugins/Rootly/v1/metadata.json

View check run for this annotation

Claude / Claude Code Review

Missing documentation link and README 

🟣 **Pre-existing nit:** The Rootly plugin has no `docs/README.md` and `metadata.json` only includes a `source` link — no `documentation` link. The repo guidelines say `links` should "typically contain two" (source and documentation), and the documentation link is what powers the "Need help?" panel in the SquaredUp configuration UI (handy for telling users where to obtain a Rootly API key). This predates the PR and is not blocking — entirely optional follow-up, since adding it would mean authorin
Comment on lines 23 to 28

@claude claude Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 🟣 Pre-existing nit: The Rootly plugin has no docs/README.md and metadata.json only includes a source link — no documentation link. The repo guidelines say links should "typically contain two" (source and documentation), and the documentation link is what powers the "Need help?" panel in the SquaredUp configuration UI (handy for telling users where to obtain a Rootly API key). This predates the PR and is not blocking — entirely optional follow-up, since adding it would mean authoring new documentation content rather than a one-line tweak.

Extended reasoning...

What's missing

plugins/Rootly/v1/ contains metadata.json, ui.json, icon.png, dataStreams/, and cspell.json — no docs/ directory and no README.md anywhere under the plugin. Correspondingly, the links array in metadata.json (lines 30–36 of the file post-PR) contains a single entry with category: source and no entry with category: documentation.

The repo's plugin guidelines describe links as: "Should typically contain two links, one link with category: source linking to the GitHub repository, and another link with category: documentation linking to the markdown documentation in the repository." The documentation link drives the in-product "Need help?" section shown when a user configures the plugin, so absent that link there's no place to surface guidance like "where do I get a Rootly API key?".

Why this is pre-existing and unrelated to the PR's purpose

The PR's purpose is adding offset-based paging to the incidents data stream. Its metadata.json edits are limited to (a) bumping the version 1.0.2 → 1.0.3 and (b) removing the unrecognized modules: [] key. The links array is untouched, and the PR description marks "No documentation changes needed". The docs gap existed when the plugin first shipped at v1.0.2.

Addressing the counter-arguments

A fair counter is that the guideline says "typically", not must, and Rootly isn't the only community WebAPI plugin without a documentation link — a handful of others (e.g. Phare, Postcoder) are in the same state. So this isn't a hard requirement, and the original framing of "all 18 other plugins follow this convention" was overstated. Another fair counter is that fixing this means authoring a brand-new README (API-key setup, UI fields, etc.) — that's more substantive than the previously-flagged drive-bys (version bump, tags field), and arguably scope creep for a paging PR.

Why mention it anyway

The majority of community WebAPI plugins (NinjaOne, Checkly, UptimeRobot, Huntress, Microsoft Defender, DigiCert, Google Sheets, AutoTask, DattoRMM, etc. — 14+ of them) do ship a docs/README.md and link to it, so the convention is the prevailing pattern even if not universal. Flagging it here gives the author an explicit chance to either add a short README in a follow-up PR or knowingly defer. Severity: pre_existing — definitely not blocking this PR.

Step-by-step proof

  1. ls plugins/Rootly/v1/ → cspell.json dataStreams icon.png metadata.json ui.json (no docs/, no README.md).
  2. Inspect plugins/Rootly/v1/metadata.json links array → one entry, category: source. No documentation entry.
  3. git show 656e4fa:plugins/Rootly/v1/metadata.json → original v1.0.2 commit had the same single-link links array, confirming this predates this PR.
  4. Spot-check a similar community plugin, e.g. plugins/NinjaOne/v1/metadata.json → links has two entries (source + documentation) and the plugin ships docs/README.md.

Possible fix (optional)

Add plugins/Rootly/v1/docs/README.md with a short overview, how to obtain a Rootly API key, and notes on the timestampField UI option. Then add a second entry to the links array in metadata.json:

{
    "category": "documentation",
    "url": "https://github.com/squaredup/plugins/blob/main/plugins/Rootly/v1/docs/README.md",
    "label": "Documentation"
}

Equally reasonable to skip and address in a dedicated documentation PR.

@clarkd clarkd Jun 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional add here if you like @noorulkhan-n but not required for this PR.

},
"links": [
{
Expand Down
Loading