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

Add support for --check flag in update command #62

Merged
merged 3 commits into from
Aug 10, 2024

Conversation

Midnightific
Copy link
Contributor

Closes: #47

Implements support for a --check flag in the update command to check if there are any updates for installed tools but will not update the tools in question

Test used:

  1. Make a random folder
  2. Run rokit init
  3. Change version of package(s) to an outdated version
    3.1 Used Rojo version 7.4.1 to check for updates (expected to resolve version 7.4.2)
    3.2 Used wally version 0.3.1 to check for updates (expected to resolve version 0.3.2)

Test resolved both of the correct up to date versions :)

I'm new to rust so any feedback is appreciated! 🙃

Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I'd like to see two changes before merging this:

  • The new pub check: bool needs a code comment above it so that documentation shows up when running rokit update --help
  • We should probably do an early return instead of if self.check { /* print versions logic */ } else { /* update versions logic */ }, it will make the diff cleaner and keeps the spirit of the new arg a bit better (--check = exit early, exit after check)

Everything else looks good!

Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

One last thing - it seems like a comment got removed accidentally.

// FUTURE: Install the newly updated tools automatically

We should keep this since the update command does not do this yet.

@filiptibell filiptibell merged commit 90235f9 into rojo-rbx:main Aug 10, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a flag to check for tool updates, without installing them
2 participants