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

Initial draft of profile metadata format & CLI #17

Merged
merged 16 commits into from
Mar 29, 2022
Merged

Initial draft of profile metadata format & CLI #17

merged 16 commits into from
Mar 29, 2022

Conversation

enderger
Copy link
Contributor

@enderger enderger commented Feb 23, 2022 •
edited
Loading

Initial branch for profile metadata

Resolves #8

@enderger enderger added this to the Initial Launcher Release milestone Feb 23, 2022
@enderger enderger self-assigned this Feb 23, 2022
Geometrically
Geometrically requested changes Feb 23, 2022
View reviewed changes
Copy link
Member

@Geometrically Geometrically left a comment

Choose a reason for hiding this comment

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

LGTM so far, besides my comment

theseus/Cargo.toml Outdated Show resolved Hide resolved
@enderger
Copy link
Contributor Author

Are there any missing fields in the profile metadata?

@Geometrically
Copy link
Member

Not that I can see of (besides the mods list, which is a separate issue)

@enderger enderger changed the title DRAFT: Initial draft of profile metadata format DRAFT: Initial draft of profile metadata format & CLI Mar 21, 2022
@enderger enderger marked this pull request as ready for review March 21, 2022 15:55
@enderger
Copy link
Contributor Author

Alright, this should be ready for review. The core profile system is implemented, however mod management is not there yet. Merging due to meeting discussion, so that profiles and the CLI can be developed in tandem.

@enderger enderger changed the title DRAFT: Initial draft of profile metadata format & CLI Initial draft of profile metadata format & CLI Mar 22, 2022
@enderger
Copy link
Contributor Author

For reference, indentation changes were done due to cargo fmt

Geometrically
Geometrically requested changes Mar 23, 2022
View reviewed changes
Copy link
Member

@Geometrically Geometrically left a comment

Choose a reason for hiding this comment

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

Love the PR~ Great work on the semaphores for limiting downloads. Mostly just small changes/nitpicks. Also, could we make the concurrent downloads limit be configurable (if it isn't already)?

theseus/src/data/profiles.rs Outdated Show resolved Hide resolved
theseus/src/launcher/java.rs Show resolved Hide resolved
theseus/src/launcher/mod.rs Outdated Show resolved Hide resolved
@enderger
Copy link
Contributor Author

enderger commented Mar 23, 2022 •
edited
Loading

Love the PR~ Great work on the semaphores for limiting downloads. Mostly just small changes/nitpicks. Also, could we make the concurrent downloads limit be configurable (if it isn't already)?

It is set by a constant, as I was unsure of how it would best be configured. Probably could put it in settings, though that might make semaphore initialization less than constant. Otherwise, representing downloaders as a pool with a semaphore could work.

@Geometrically
Copy link
Member

Love the PR~ Great work on the semaphores for limiting downloads. Mostly just small changes/nitpicks. Also, could we make the concurrent downloads limit be configurable (if it isn't already)?

It is set by a constant, as I was unsure of how it would best be configured. Probably could put it in settings, though that might make semaphore initialization less than constant. Otherwise, representing downloaders as a pool with a semaphore could work.

You could make the semaphore a oncecell like the other "singletons" and have it initialized after settings. It would be really useful if it was configurable

Geometrically
Geometrically requested changes Mar 23, 2022
View reviewed changes
Copy link
Member

@Geometrically Geometrically left a comment

Choose a reason for hiding this comment

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

Oops, accidentally approved

@enderger enderger mentioned this pull request Mar 24, 2022
Closed
@triphora triphora linked an issue Mar 24, 2022 that may be closed by this pull request
PathBuf#set_extension messes up version names #24
Closed
Geometrically
Geometrically approved these changes Mar 29, 2022
View reviewed changes
Copy link
Member

@Geometrically Geometrically left a comment

Choose a reason for hiding this comment

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

LGTM! Great work on the PR.

@Geometrically Geometrically merged commit d1070ca into modrinth:master Mar 29, 2022
enderger added a commit to enderger/theseus that referenced this pull request Jun 25, 2022
@enderger
Initial draft of profile metadata format & CLI (modrinth#17)
ef82d34 
* Initial draft of profile metadata format

* Remove records, add Clippy to Nix, fix Clippy error

* Work on profile definition

* BREAKING: Make global settings consistent with profile settings

* Add builder methods & format

* Integrate launching with profiles

* Add profile loading

* Launching via profile, API tweaks, and yak shaving

* Incremental update, committing everything due to personal system maintainance

* Prepare for review cycle

* Remove reminents of experimental work

* CLI: allow people to override the non-empty directory check

* Fix mistake in previous commit

* Handle trailing whitespace and newlines in prompts

* Revamp prompt to use dialoguer and support defaults

* Make requested changes
Geometrically added a commit that referenced this pull request Jul 4, 2024
@CodexAdrian @Geometrically
Modal report (#17)
4ae7786 
* Base modal implementation

* Modal Report page

* Upgrade multiselect

* Fixed multiselect styling

* fix build err

* rev change

* Added dropdown component, addressed changes

Removed unused classes after vue multiselect was removed
Updated markdown-it and xss

* Update index.js

* fix lint

* Fix prettier code style

* Address most changes

* New dropdown

* Undo comment

Makes the component close when not focused

* Fix accessibility issues

* Fix double focus

* addressed changes to modal

* Run Prettier

* Update ModalReport.vue

* Fixed spacing issues

---------

Co-authored-by: Jai A <jaiagr+gpg@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Geometrically Geometrically Geometrically approved these changes

Assignees

@enderger enderger

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PathBuf#set_extension messes up version names Theseus: Profile Management
2 participants
@enderger @Geometrically