Skip to content
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

Jump to bottom

feat: support multiple configs #588

Merged
merged 5 commits into from
Feb 28, 2025
Merged

feat: support multiple configs #588

merged 5 commits into from
Feb 28, 2025

Conversation

AtomicFS
Copy link
Collaborator

@AtomicFS AtomicFS commented Feb 19, 2025 •
edited
Loading

fixes #559

@github-actions github-actions bot added documentation Improvements or additions to documentation feature New feature or request github_actions Pull requests that update GitHub Actions code labels Feb 19, 2025
@AtomicFS AtomicFS force-pushed the feat/support-multiple-configs branch 3 times, most recently from 7457c13 to 0cb24ad Compare February 20, 2025 17:56
@AtomicFS AtomicFS marked this pull request as ready for review February 20, 2025 18:00
@AtomicFS AtomicFS requested a review from MDr164 as a code owner February 20, 2025 18:00
@AtomicFS AtomicFS enabled auto-merge February 20, 2025 18:03
@AtomicFS AtomicFS force-pushed the feat/support-multiple-configs branch 3 times, most recently from bb04108 to 990346b Compare February 21, 2025 12:14
@AtomicFS AtomicFS added this to the v0.15.0 release milestone Feb 21, 2025
MDr164
MDr164 previously approved these changes Feb 21, 2025
View reviewed changes
Copy link
Collaborator

@MDr164 MDr164 left a comment

Choose a reason for hiding this comment

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

I still think simply hashing would have been enough and that this is a bit over-engineered but it works so who am I to judge

@AtomicFS
Copy link
Collaborator Author

I think you mean #592, that is the one for change detection.

@AtomicFS AtomicFS disabled auto-merge February 22, 2025 06:45
@AtomicFS AtomicFS force-pushed the feat/support-multiple-configs branch from 990346b to 566046e Compare February 24, 2025 13:55
@github-actions github-actions bot added the testing Testing related label Feb 24, 2025
@AtomicFS AtomicFS enabled auto-merge February 24, 2025 13:55
@AtomicFS
Copy link
Collaborator Author

AtomicFS commented Feb 24, 2025 •
edited
Loading

Well, since I had hard time with reflections, I had some assist from AI. And while doing that I decided to bring #501 to a close, updating CONTRIBUTING.md with instructions on AI. Sorry for bundling it together like so, but seems easier. I decided to go with git trailer style.

Also, I feel kinda lazy to try and cleanup the commit history. But if you @MDr164 would prefer to have it cleaner, let me know and I will merge some of the commits.

**UPDATE: moved to #600 **

@AtomicFS AtomicFS requested a review from MDr164 February 24, 2025 14:08
@AtomicFS AtomicFS force-pushed the feat/support-multiple-configs branch 2 times, most recently from d43f185 to ba5ec0d Compare February 24, 2025 14:12
MDr164
MDr164 reviewed Feb 28, 2025
View reviewed changes
Copy link
Collaborator

@MDr164 MDr164 left a comment

Choose a reason for hiding this comment

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

Reading the reflection code gave me an aneurysm but it looks like it is working as intended 👍

@AtomicFS
feat(cmd): support multiple configuration files
7926245 
- this should hopefully help to reduce copy-pasting for users
- I just changed 2 example test to test this feature
- the difference is order of the configuration files, for coreboot the
  target is in first config, while for linux it is in the second

Signed-off-by: AtomicFS <[email protected]>
@AtomicFS
refactor(cmd): use reflection in AllModules method
478cac3 
- replace hard-coded code with reflection to make it more flexible

AI-Generated: true
AI-Model: ChatGPT o3-mini
Signed-off-by: AtomicFS <[email protected]>
@AtomicFS
test(cmd): use reflection in AllModules method
05d32ed 
- add unit-test to validate functionality

Signed-off-by: AtomicFS <[email protected]>
@AtomicFS
refactor(cmd): use reflection in Merge method
fa9d06b 
- this will make it more flexible when we add more modules in the future
- also add unit-test

AI-Generated: true
AI-Model: ChatGPT o3-mini
Signed-off-by: AtomicFS <[email protected]>
@AtomicFS
docs: support multiple configuration files
c56c147 
Signed-off-by: AtomicFS <[email protected]>
@AtomicFS AtomicFS force-pushed the feat/support-multiple-configs branch from ba5ec0d to c56c147 Compare February 28, 2025 16:58
@AtomicFS AtomicFS requested a review from MDr164 February 28, 2025 17:10
@AtomicFS AtomicFS added this pull request to the merge queue Feb 28, 2025
Merged via the queue into main with commit e737afc Feb 28, 2025
97 of 107 checks passed
@AtomicFS AtomicFS deleted the feat/support-multiple-configs branch February 28, 2025 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MDr164 MDr164 MDr164 approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation feature New feature or request github_actions Pull requests that update GitHub Actions code testing Testing related
Projects
None yet
Milestone
v0.15.0 release
Development

Successfully merging this pull request may close these issues.

allow use of multiple configuration files
2 participants
@AtomicFS @MDr164