feat: Allow installing PRs from DeveloperView #139
Conversation
Hardcoded for launcher right now
using `ts-rs` as done in #140
What is returned is actually an array of `PullsApiResponseElement`
Used Mods instead of Launcher
NorthstarLauncher vs NorthstarMods
for checking game path
|
Ok this is nearing completion now. Only thing missing is reducing duplicate code, around launcher vs mod, i.e. use an enum to discern the two and then have a single function to handle both. |
So that all constants are in a single place
There was a problem hiding this comment.
Before reviewing in details, I have a few questions:
- Why can't I install some PRs? ("Couldn't grab download link for PR 414" isn't an explicit error)
- Can we prevent installing several times the same PR?
- While installing mods PRs, instead of forcing users to go outside FlightCore to switch profile and actually test them, we should handle the profile switch ourselves, shall we?
- Do you want me to improve UI a bit?
I guess no downloadable CI build. Need to investigate further
We could but that would require tracking which PR is installed right now which would be a lot more additional logic that (in my opinion) would add rather little value. At least to me there's no point in preventing installing the same PR multiple times. ^^
Would require fully supporting profiles first, which we don't do yet. Next best solution would be having a button to launch Overall, same with the previous question, the goal of this PR is to achieve feature parity with https://github.com/GeckoEidechse/northstar_dev_testing_helper_tool so that I can deprecate that. The PR in it's current state has already achieved that minus search filter, which leads me to:
If you don't mind <3 |
Displays a notification that PR install has started and upon completion replaces it with one saying that install has completed.
For now I added a notification similar to #154 where a notification is displayed once installation starts and removed again upon completion and replaced with one that says installation has completed. With that out of the way, I'd say this PR is ready to merge and I'd turn the last TODO into an issue to solve post-merge. The reason for this being that the branch is starting to diverge more and more and it's already become annoying to resolve merge conflicts every time another PR gets merged. Additionally this feature is already on par with https://github.com/GeckoEidechse/northstar_dev_testing_helper_tool/ and it's too useful to keep delaying it for polish IMO ^^ |
* refactor: export pull requests selector to dedicated component * refactor: regroup launcher+mods collapses in one collapse component * refactor: load pull requests when opening selector collapse item * refactor: review progress loaders' style * fix: don't fetch PRs if they've already been loaded * feat: update collapse style * refactor: remove fetch success notification * refactor: both collapses can be opened at the same time * fix: non-accordion collapse sends an object as event parameter
This ensures we can still grab older artifacts. Max page is capped at 10 as going too high will cause us to hit API rate limits. Also refined error message accordingly.
There was a problem hiding this comment.
We should create a store module dedicated to PR installation (just like searchModule module) to keep code clean.
Otherwise, I confirm code looks great and c4fc200 fixed related issue; I'll be happy to merge once comments have been addressed :)
as suggested in review
I order to clean up PullRequestSelector.vue Other functions will follow in separate commits. Currently TypeScript compilation fails on undefined type of `state`
Allows installing launcher and mod PRs from within FlightCore, essentially replacing https://github.com/GeckoEidechse/northstar_dev_testing_helper_tool/
TODO
constants.rsBetter error messages when PR couldn't be downloaded (e.g. Update libcurl and remove openssl dependency R2Northstar/NorthstarLauncher#414)Solved by fixing underlying issue in c4fc200Requires: