Skip to content

Conversation

@stefpiatek
Copy link
Contributor

@stefpiatek stefpiatek commented Jan 13, 2026

And also quote token so whitespace is clear

@stefpiatek stefpiatek requested a review from milanmlft January 13, 2026 17:16
@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.54%. Comparing base (186afc9) to head (f109dbd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #620   +/-   ##
=======================================
  Coverage   87.54%   87.54%           
=======================================
  Files          77       77           
  Lines        3694     3694           
=======================================
  Hits         3234     3234           
  Misses        460      460           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stefpiatek stefpiatek requested a review from p-j-smith January 14, 2026 09:31
Copy link
Contributor

@p-j-smith p-j-smith left a comment

Choose a reason for hiding this comment

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

Looks good, a couple of questions:

  • the three configs look the same to me (except for the name), just wondering why there can't be a single config / project?
  • unrelated to the PR, but as the number of configs grows, should we make sub-directories for them (e.g. configs/test, configs/education, configs/production`)?

@stefpiatek
Copy link
Contributor Author

Looks good, a couple of questions:

* the three configs look the same to me (except for the name), just wondering why there can't be a single config / project?

Yeah they've got separate TRE workspaces, which is frustrating

* unrelated to the PR, but as the number of configs grows, should we make sub-directories for them (e.g. `configs/test`, `configs/education`, configs/production`)?

Not a bad shout, I think for that we'd need to double check that we're still checking all configurations and have a different method for defining which subdirectory to look in. What might be an idea is to just move "done" projects to an archive subdirectory?

@stefpiatek stefpiatek merged commit c505ecf into main Jan 14, 2026
19 of 20 checks passed
@stefpiatek stefpiatek deleted the stefpiatek/add_ngtube_education_projects branch January 14, 2026 10:51
@p-j-smith
Copy link
Contributor

I think for that we'd need to double check that we're still checking all configurations and have a different method for defining which subdirectory to look in.

Ah I hadn't thought of that. We could do Path(config("PROJECT_CONFIGS_DIR")).rglob(f"{project_slug}.yaml") so it doesn't matter which subdirectory it's in. But yeah having an archive directory is a good call

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.

3 participants