Skip to content

docs(adrs): Add ADRs for DLT and ELT#373

Open
WHTaylor wants to merge 4 commits into
mainfrom
elt_adrs
Open

docs(adrs): Add ADRs for DLT and ELT#373
WHTaylor wants to merge 4 commits into
mainfrom
elt_adrs

Conversation

@WHTaylor

@WHTaylor WHTaylor commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

ref #121

Fixes the ADR directory config (looks like they were moved in #255), adds missing ADRs to the index, then adds an ADR about using DLT, and another one superceding it.

  • The DLT one is pretty vague because I had nothing to do with the decision 😅 - if you think there's anything important to add, please say @martyngigg
  • Should either of these be backdated?

Summary by CodeRabbit

  • Documentation
    • Updated the architecture decision record index with the latest entries and statuses.
    • Added two new architecture decision records covering the evolution of the ingestion approach.
    • Corrected the architecture documentation path configuration so it points to the current location.

@WHTaylor WHTaylor requested a review from a team as a code owner June 25, 2026 17:44
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The ADR directory path was updated, two new ADRs were added for data-load-tool ingestion and its replacement, and the ADR index was expanded to list ADRs 0001–0011 with updated statuses.

Changes

ADR catalogue refresh

Layer / File(s) Summary
ADR path and catalogue entries
.adr-dir, docs-devel/system-architecture/adrs/0010-use-data-load-tool-for-ingesting-data.md, docs-devel/system-architecture/adrs/0011-replace-dlt-in-ingestion-pipelines.md, docs-devel/system-architecture/adrs/index.md
The ADR root path moved, ADR 0010 and 0011 were added, and the index table now lists ADRs 0001–0011 with updated statuses.

Poem

A rabbit hopped through ADR glen,
With index pages now and then.
One path moved left, two notes were new,
The burrow hummed: “thump, tidy, true!”
🐇📘

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: adding ADRs for DLT and its replacement with ELT.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@docs-devel/system-architecture/adrs/0011-replace-dlt-in-ingestion-pipelines.md`:
- Line 9: The ADR supersession wording is incorrect in this document and should
use the standard term consistently. Update the supersession reference in this
ADR to “Supersedes” and ensure the paired 0010 ADR uses “Superseded” so the
relationship is expressed with the correct canonical wording.

In `@docs-devel/system-architecture/adrs/index.md`:
- Around line 19-20: The ADR index labels are using five-digit IDs for these
entries, which is inconsistent with the rest of the catalogue and the linked
filenames. Update the table entries in the ADR index so the displayed IDs match
the zero-padded four-digit format used elsewhere, and keep the link targets
unchanged; use the existing ADR list/table markup in index.md to locate the
affected rows.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1985b3be-a5a2-4a05-b5f5-100a0c88b186

📥 Commits

Reviewing files that changed from the base of the PR and between 857eba7 and e8f3ea2.

📒 Files selected for processing (4)
  • .adr-dir
  • docs-devel/system-architecture/adrs/0010-use-data-load-tool-for-ingesting-data.md
  • docs-devel/system-architecture/adrs/0011-replace-dlt-in-ingestion-pipelines.md
  • docs-devel/system-architecture/adrs/index.md


Accepted

Supercedes [10. Use data load tool for ingesting data](0010-use-data-load-tool-for-ingesting-data.md)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use standard supersession wording.

Supercedes should be Supersedes here; the paired 0010 ADR should say Superseded as well.

📝 Proposed fix
- Supercedes [10. Use data load tool for ingesting data](0010-use-data-load-tool-for-ingesting-data.md)
+ Supersedes [10. Use data load tool for ingesting data](0010-use-data-load-tool-for-ingesting-data.md)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Supercedes [10. Use data load tool for ingesting data](0010-use-data-load-tool-for-ingesting-data.md)
Supersedes [10. Use data load tool for ingesting data](0010-use-data-load-tool-for-ingesting-data.md)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs-devel/system-architecture/adrs/0011-replace-dlt-in-ingestion-pipelines.md`
at line 9, The ADR supersession wording is incorrect in this document and should
use the standard term consistently. Update the supersession reference in this
ADR to “Supersedes” and ensure the paired 0010 ADR uses “Superseded” so the
relationship is expressed with the correct canonical wording.

Comment on lines +19 to +20
| [00010](./0010-use-data-load-tool-for-ingesting-data.md) | Use data load tool for ingesting data | Superseded |
| [00011](./0011-replace-dlt-in-ingestion-pipelines.md) | Replace DLT in ingestion pipelines | Accepted |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep the ADR IDs zero-padded to four digits.

00010/00011 are inconsistent with the rest of the catalogue and the filenames (0010/0011). The links work, but the labels will mislead anyone scanning by ADR number.

📝 Proposed fix
-| [00010](./0010-use-data-load-tool-for-ingesting-data.md)            | Use data load tool for ingesting data                  | Superseded |
-| [00011](./0011-replace-dlt-in-ingestion-pipelines.md)               | Replace DLT in ingestion pipelines                     | Accepted   |
+| [0010](./0010-use-data-load-tool-for-ingesting-data.md)             | Use data load tool for ingesting data                  | Superseded |
+| [0011](./0011-replace-dlt-in-ingestion-pipelines.md)                | Replace DLT in ingestion pipelines                     | Accepted   |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| [00010](./0010-use-data-load-tool-for-ingesting-data.md) | Use data load tool for ingesting data | Superseded |
| [00011](./0011-replace-dlt-in-ingestion-pipelines.md) | Replace DLT in ingestion pipelines | Accepted |
| [0010](./0010-use-data-load-tool-for-ingesting-data.md) | Use data load tool for ingesting data | Superseded |
| [0011](./0011-replace-dlt-in-ingestion-pipelines.md) | Replace DLT in ingestion pipelines | Accepted |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs-devel/system-architecture/adrs/index.md` around lines 19 - 20, The ADR
index labels are using five-digit IDs for these entries, which is inconsistent
with the rest of the catalogue and the linked filenames. Update the table
entries in the ADR index so the displayed IDs match the zero-padded four-digit
format used elsewhere, and keep the link targets unchanged; use the existing ADR
list/table markup in index.md to locate the affected rows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant