fricklerhandwerk/Fediversity
1
0
Fork 0
forked from fediversity/fediversity
Code Pull requests Activity

data model: add classes #7

Closed
kiara wants to merge 1 commit from data-model-add-classes into deployment-data-model-with-tests
pull from: data-model-add-classes
merge into: fricklerhandwerk:deployment-data-model-with-tests
Conversation 2 Commits 1 Files changed 1 +147 -119
kiara commented 2025-07-31 17:30:09 +02:00
Contributor
Copy link

specifies classes in the data model, raising these to from _class model attributes to subModuleWith's class parameter where possible.

note: resource policy has remained as to use deferredModuleWith (rather than submoduleWith) to ensure evaluation will go thru.

may subsume #6, in addition to superseding #4.

merge-wise, it may be preferable to handle this PR before #5.

finally, while forgejo is horrible in viewing diffs, the likes of diffchecker handle this slightly better, highlighting changed parts of lines. in practice tho, there aren't really additional changes here.

specifies classes in the data model, raising these to from `_class` model attributes to `subModuleWith`'s `class` parameter where possible. note: resource policy has remained as to use `deferredModuleWith` (rather than `submoduleWith`) to ensure evaluation will go thru. may subsume #6, in addition to superseding #4. merge-wise, it may be preferable to handle this PR before #5. finally, while forgejo is horrible in viewing diffs, the likes of [diffchecker](https://www.diffchecker.com/text-compare/) handle this slightly better, highlighting changed parts of lines. in practice tho, there aren't really additional changes here.
fricklerhandwerk reviewed 2025-07-31 18:06:12 +02:00
deployment/data-model.nix Outdated
@ -132,2 +139,2 @@
description = ''
Resources made available by the hosting provider, and their policies.
type = attrsOf (submoduleWith {
class = "fediversity-environment";
fricklerhandwerk commented 2025-07-31 18:06:12 +02:00
Owner
Copy link

I don't think this is an improvement in readability

I don't think this is an improvement in readability
kiara commented 2025-07-31 18:29:39 +02:00
Author
Contributor
Copy link

the PR's intent was semantic rather than just documentation - even if (as noted in Fediversity/Fediversity#481 (comment)) i haven't fully grasped the full extent of subModuleWith's class parameter.

the PR's intent was semantic rather than just documentation - even if (as noted in https://git.fediversity.eu/Fediversity/Fediversity/pulls/481#issuecomment-9183) i haven't fully grasped the full extent of `subModuleWith`'s `class` parameter.
kiara commented 2025-08-17 20:33:47 +02:00
Author
Contributor
Copy link

looks like a lot of that pertains to merging logic - maybe the type checks are the main thing then

looks like a lot of that pertains to merging logic - maybe the type checks are the main thing then
fricklerhandwerk reviewed 2025-07-31 18:06:30 +02:00
deployment/data-model.nix Outdated
@ -31,6 +33,7 @@ let
inputs.nixops4.modules.nixops4Deployment.default
{
_class = "nixops4Deployment";
fricklerhandwerk commented 2025-07-31 18:06:30 +02:00
Owner
Copy link

Very likely not needed, as noted in the other PR

Very likely not needed, as noted in the other PR
kiara commented 2025-07-31 18:22:43 +02:00
Author
Contributor
Copy link

resolving until the upstream PR may supersede this, now #6 is merged

resolving until the upstream PR may supersede this, now #6 is merged
kiara commented 2025-07-31 20:56:39 +02:00
Author
Contributor
Copy link

given this got merged upstream, i rebased to remove this now, ahead of our updating nixops4 (which i will consider out of scope for this PR, as i'm sure we will down the line)

given this got merged upstream, i rebased to remove this now, ahead of our updating nixops4 (which i will consider out of scope for this PR, as i'm sure we will down the line)
kiara marked this conversation as resolved
fricklerhandwerk reviewed 2025-07-31 18:07:13 +02:00
fricklerhandwerk left a comment
Owner
Copy link

The small changes dotting the i-s make sense, thanks a lot! The larger one not so much.

The small changes dotting the i-s make sense, thanks a lot! The larger one not so much.
kiara force-pushed data-model-add-classes from 6cba217e53 to 9f596ad820 2025-07-31 20:55:33 +02:00 Compare
kiara commented 2025-08-01 11:43:38 +02:00
Author
Contributor
Copy link

hm, while submoduleWith seems to actually enforce the check, i'm now not sure deferredModuleWith and submodule function as more than documentation on this.

hm, while `submoduleWith` seems to actually enforce the check, i'm now not sure `deferredModuleWith` and `submodule` function as more than documentation on this.
kiara closed this pull request 2025-08-27 00:45:58 +02:00
kiara commented 2025-08-27 13:03:17 +02:00
Author
Contributor
Copy link

superseded by Fediversity/Fediversity#504

superseded by https://git.fediversity.eu/Fediversity/Fediversity/pulls/504

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: fricklerhandwerk/Fediversity#7
No description provided.
Delete branch "data-model-add-classes"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?