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.

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

meta: add Profile management #444

Closed
wants to merge 11 commits into from
Closed

meta: add Profile management #444

wants to merge 11 commits into from

Conversation

Jan200101
Copy link
Contributor

@Jan200101 Jan200101 commented Jul 23, 2023

meta PR to keep all the not yet PRable commits for profile management.

each and every one is going to be PR'd independenly and the profiles branch will get rebased after a merge.

@Jan200101 Jan200101 changed the title WIP: profiles feat(WIP): profiles Jul 23, 2023
@GeckoEidechse
Copy link
Member

GeckoEidechse commented Jul 23, 2023

Holy fuck this is so cool :DDDDD

Good thing I got distracted with working on some other FlightCore feature today cause otherwise we would have done the same work at once lmao xD

src-tauri/src/northstar/profile.rs Outdated Show resolved Hide resolved
src-tauri/src/northstar/profile.rs Outdated Show resolved Hide resolved
src-tauri/src/northstar/profile.rs Outdated Show resolved Hide resolved
@Jan200101
Copy link
Contributor Author

Good thing I got distract with working on some other FlightCore feature today cause otherwise we would have done the same work at once lmao xD

It would be fine, two people working on the same thing would give you two unique perspectives on the implementation, allowing you to cherry pick the best of both worlds :)

@Jan200101 Jan200101 changed the title feat(WIP): profiles feat: add Profile management Jul 28, 2023
@Jan200101
Copy link
Contributor Author

for the sake of not making this an absolute nightmare, I'll split this up into chunks and make new PRs for them.

GeckoEidechse pushed a commit that referenced this pull request Aug 3, 2023
Passes the whole `GameInstall` object instead of individual path to functions related to PR installs.
This is done in preparation for #444
@Jan200101
Copy link
Contributor Author

This isn't the end of it, but all front facing functionality that would need to support Profiles is done.

Things to get done in the future:

  • Migrate Launcher PRs and Mod PRs to the Profile system
  • Fresh Profile creation

@Jan200101 Jan200101 marked this pull request as ready for review August 4, 2023 17:44
Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

So happy to see the progress :D

Left a few small comments.

With that said, I think we should try to still further reduce the diff a bit. In particular, as a next step I would splice out the Rust code for profile fetching/validation into a separate PR.
Add some buttons to test it in DevView and then we can merge that early as well.

After that it might also make sense to have the profile selection field in DevView for a bit so that we can make a release where contributors can basically test it in production.

Once it's considered stable we would just move the controls to settings for actual "release" ^^

src-vue/src/views/SettingsView.vue Outdated Show resolved Hide resolved
src-tauri/src/northstar/profile.rs Outdated Show resolved Hide resolved
src-tauri/src/northstar/profile.rs Outdated Show resolved Hide resolved
@GeckoEidechse GeckoEidechse added the enhancement New feature or request label Aug 6, 2023
@Jan200101
Copy link
Contributor Author

squashed all commits
rebased onto master
separated every pragtical addition into separate commits to easily digest.

every single commit was tested to work (on Linux) individually and ran against clippy and the formatter.

This should make a review more feasible to do.

@GeckoEidechse
Copy link
Member

You're a saint. Will take a look tonight, thanks <3

@Jan200101
Copy link
Contributor Author

little issue I have found:
switching profiles does not re-check the installation.

@GeckoEidechse
Copy link
Member

Didn't get to it, sorry :/

Given the nice commit structure, my plan is to just cherry-pick a the first few changes into separate PRs, so if you wanna get ahead of me feel free to just cherry-pick the first commit into a new PR and I will hopefully get to it tomorrow <3

@Jan200101
Copy link
Contributor Author

#494

@Jan200101 Jan200101 changed the title feat: add Profile management meta: add Profile management Aug 8, 2023
@Jan200101 Jan200101 marked this pull request as draft August 8, 2023 22:06
GeckoEidechse added a commit that referenced this pull request Oct 9, 2023
This adds a button and corresponding logic to delete an existing profile

Spliced out from #444

Co-authored-by: Jan <[email protected]>
GeckoEidechse added a commit that referenced this pull request Oct 10, 2023
This adds a button and corresponding logic to delete an existing profile

Spliced out from #444

Co-authored-by: Jan <[email protected]>
@github-actions github-actions bot added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Jan 17, 2024
@GeckoEidechse GeckoEidechse removed the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Jan 19, 2024
@github-actions github-actions bot added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Jan 21, 2024
@GeckoEidechse
Copy link
Member

Superseded by #761

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request merge conflicts Blocked by merge conflicts, waiting on the author to resolve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants