-
-
Notifications
You must be signed in to change notification settings - Fork 737
Support launching app with file for win and linux #648
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
Support launching app with file for win and linux #648
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. There's an open PR that converts all the singletons into interfaces that will need updating after this one as it will conflict but that isn't this PRs problem.
/// <summary> | ||
/// Gets or sets a value representing Chrome's version string. | ||
/// </summary> | ||
public string Chrome { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are read only values you might make the set internal for these two props.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, i figured this would be a perfect use case for using a record type to achieve the desired read-only behavior. See my latest commit.
I have re-run all my tests and encountered no issue with this change. 👍
…ly access to props This is to address a PR ElectronNET#648 review comment to ensure that only the external users are not able to modify the instance values.
…adonly access to props This is to address a PR ElectronNET#648 review comment to ensure that only the external users are not able to modify the instance values.
Status: Review Approved
Summary
Addresses #647 and remains compatible with existing behavior of passing arguments via /args commandline option.
This has been built and integration tested locally by invoking via electronize start as well as the compiled executable as described in the issue #647.
Change Summary
Verification Summary
I was not able to get the WebApp project running however I was able to get my target project running with the update API & CLI changes. I created a simple test method as follows:
The generated the following results with no issues.
Results: