Resolve merge conflicts from pr-38 - #41
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR resolves merge conflicts from PR-38 by aligning documentation with the codebase, fixing quoting issues in the update script, and ensuring proper file ownership and screen directory persistence across all installation methods.
- Fixed API endpoint URL quoting in the PaperMC update script to ensure proper version resolution
- Added comprehensive ownership management and screen directory persistence to all setup scripts
- Updated documentation to reflect current installer behavior including checksum validation and screen tmpfiles handling
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| update.sh | Fixed URL quoting for PaperMC API version endpoint |
| setup_minecraft_lxc.sh | Added ownership management for minecraft user |
| setup_minecraft.sh | Added comprehensive update script generation and ownership management |
| setup_bedrock.sh | Added screen directory persistence via systemd-tmpfiles |
| SIMULATION.md | Updated documentation to reflect current installer behavior |
| README.md | Aligned section headings with emoji format and added support section |
| CONTRIBUTING.md | Minor documentation improvement for command substitution style |
Comment on lines
+99
to
+101
| E2 | ||
| chmod +x update.sh | ||
|
|
There was a problem hiding this comment.
The update script is duplicated between the main update.sh file and the generated version in setup_minecraft.sh. This creates maintenance burden as changes need to be applied in two places. Consider extracting the script content to a shared template or having the setup script copy from the existing update.sh file.
Suggested change
| E2 | |
| chmod +x update.sh | |
| # Install update.sh from source to /opt/minecraft | |
| cp /path/to/source/update.sh /opt/minecraft/update.sh | |
| chmod +x /opt/minecraft/update.sh | |
| echo "✅ Installed update.sh to /opt/minecraft/update.sh" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68e917811974833393796408b8734df6