johnride
  • Joined on 2024-02-06
johnride pushed to feat/basicCI at NationTech/harmony 2025-05-28 19:19:48 +00:00
5aa9dc701f fix: Removed forgotten refactoring bits and formatting
johnride pushed to feat/basicCI at NationTech/harmony 2025-05-28 18:40:32 +00:00
f4ef895d2e feat: Add basic CI configuration
johnride created branch feat/basicCI in NationTech/harmony 2025-05-28 18:40:31 +00:00
johnride deleted branch adr/multi-tenancy from NationTech/harmony 2025-05-26 20:26:39 +00:00
johnride pushed to master at NationTech/harmony 2025-05-26 20:26:39 +00:00
6e7148a945 Merge pull request 'adr: Add ADR on multi tenancy using namespace based customer isolation' (#41) from adr/multi-tenancy into master
83453273c6 adr: Add ADR on multi tenancy using namespace based customer isolation
Compare 2 commits »
johnride merged pull request NationTech/harmony#41 2025-05-26 20:26:37 +00:00
adr: Add ADR on multi tenancy using namespace based customer isolation
johnride commented on pull request NationTech/harmony#41 2025-05-26 18:05:22 +00:00
adr: Add ADR on multi tenancy using namespace based customer isolation

I like this solution for now, I think the best solution would be a dedicated cluster per tenant but it makes sense why we can't. I think the proposed solution works for our current scale.…

johnride suggested changes for NationTech/harmony#40 2025-05-26 16:01:57 +00:00
feat/teams-integration

You missed the point of using a trait here. See my comment and refactor accordingly.

johnride commented on pull request NationTech/harmony#40 2025-05-26 16:01:57 +00:00
feat/teams-integration

This defeats the purpose of using a trait. Also prevents extensibility by external users of Harmony.

johnride created pull request NationTech/harmony#41 2025-05-26 15:57:24 +00:00
adr: Add ADR on multi tenancy using namespace based customer isolation
johnride pushed to adr/multi-tenancy at NationTech/harmony 2025-05-26 15:57:09 +00:00
83453273c6 adr: Add ADR on multi tenancy using namespace based customer isolation
johnride created branch adr/multi-tenancy in NationTech/harmony 2025-05-26 15:57:08 +00:00
johnride approved NationTech/harmony#39 2025-05-22 20:07:16 +00:00
fix: make HelmRepository public
johnride commented on pull request NationTech/harmony#39 2025-05-22 20:07:05 +00:00
fix: make HelmRepository public

I feel like this should not be public. Our public APIs should go through Scores, Topologies and Inventories. What's the reasonning here?

It's exposed through the score, and if a score…

johnride suggested changes for NationTech/harmony#39 2025-05-22 20:04:50 +00:00
fix: make HelmRepository public

I feel like this should not be public. Our public APIs should go through Scores, Topologies and Inventories. What's the reasonning here?

johnride commented on pull request NationTech/harmony#38 2025-05-22 20:01:14 +00:00
feat:added Slack notifications support

Yeah, these two functions here discord_alert_builder and slack_alert_builder here should be implementations of a trait.

johnride approved NationTech/harmony#38 2025-05-22 20:01:14 +00:00
feat:added Slack notifications support

Overall seems OK. Deserves some refactoring though.

johnride commented on pull request NationTech/harmony#38 2025-05-22 20:01:14 +00:00
feat:added Slack notifications support

Since we're supporting a list, shouldn't we use filter_map instead so we can handle all the instances at once?

johnride approved NationTech/harmony#37 2025-05-21 16:34:57 +00:00
monitoringalerting

The name MonitoringAlertingStackScore bothered me all along but I couldn't figure out why. Now I did :

johnride commented on pull request NationTech/harmony#37 2025-05-15 20:33:30 +00:00
monitoringalerting

Useless comments