Skip to content

Conversation

@Online122228
Copy link

had an error when compiling that /opt/NUSGet cannot be found and therefore not be deleted. i fixed this by changing the rm -rd /opt/NUSGet line to rm -rf /opt/NUSGet since -f forces the deletion even if the file or directory does not exist and -r already includes the functionality of -d.
Furthermore i added an option to uninstall the program by running sudo make uninstall

…g able to build if /opt/NUSGet is not already done
Copilot AI review requested due to automatic review settings February 1, 2026 02:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a Makefile compilation error and adds an uninstall target. The original issue occurred when /opt/NUSGet didn't exist during installation, causing rm -rd to fail. The fix changes the flag to -rf, which forces deletion even if the directory doesn't exist.

Changes:

  • Fixed rm -rd to rm -rf in the install target to prevent errors when the directory doesn't exist
  • Added a new uninstall target to remove installed files and directories

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


uninstall:
rm -rf /opt/NUSGet
rm /usr/share/applications/NUSGet.desktop
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

The uninstall target should use the -f flag with the rm command for consistency with line 17 and to prevent errors if the file doesn't exist. This follows the same reasoning as the fix applied to line 9.

Suggested change
rm /usr/share/applications/NUSGet.desktop
rm -f /usr/share/applications/NUSGet.desktop

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

you only uninstall an installed package, so the file is definetly there

install ./packaging/NUSGet.desktop /usr/share/applications

uninstall:
rm -rf /opt/NUSGet
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

For consistency with line 9, consider adding a trailing slash to the directory path. While both forms work, consistent formatting improves maintainability.

Suggested change
rm -rf /opt/NUSGet
rm -rf /opt/NUSGet/

Copilot uses AI. Check for mistakes.
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.

1 participant