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

Update sync workflow and configuration for sandbox deployment #108

Merged
merged 1 commit into from
Feb 19, 2025
Merged

Conversation

N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Feb 19, 2025

User description

fixes .ber issue


PR Type

Enhancement, Configuration changes


Description

  • Updated sync workflow to support sandbox deployment.

  • Modified .github/sync.yml for sandbox-specific file synchronization.

  • Changed workflow trigger to release events in .github/workflows/sync.yml.

  • Updated ResourceHacker.ini with new MRU path for sandbox.


Changes walkthrough 📝

Relevant files
Configuration changes
sync.yml
Adjusted sync configuration for sandbox deployment             

.github/sync.yml

  • Removed Bearsampp-specific file synchronization configuration.
  • Added sandbox-specific file synchronization paths and repositories.
  • +9/-21   
    ResourceHacker.ini
    Updated MRU path for sandbox executable                                   

    ResourceHacker.ini

    • Updated MRU1 path to point to sandbox executable.
    +1/-1     
    Enhancement
    sync.yml
    Updated workflow trigger and name                                               

    .github/workflows/sync.yml

  • Renamed workflow to "Build and Deploy on Release".
  • Changed trigger from branch push to release events.
  • +5/-5     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @N6REJ N6REJ merged commit ed217eb into main Feb 19, 2025
    1 check passed
    @N6REJ N6REJ deleted the ber-fix branch February 19, 2025 09:12
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 Security concerns

    Sensitive information exposure:
    The ResourceHacker.ini file contains a hardcoded local development path 'E:\Bearsampp-development\sandbox\bearsampp.exe' which could expose system information. Consider using relative paths or environment variables instead.

    ⚡ Recommended focus areas for review

    File Permissions

    The sync configuration for copying executable files should specify file permissions to ensure proper execution rights are maintained during synchronization

    - source: /bin/tmp/release/bearsampp.exe
      dest: /bearsampp.exe
    Workflow Security

    The workflow is triggered on any published release. Consider restricting to specific release types or adding approval requirements for production deployments

    release:
        types: [published]

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add file existence validation

    Add file existence validation before sync to prevent potential sync failures if
    source files are missing.

    .github/sync.yml [4-8]

     - files:
           - source: /core/resources/version.dat
             dest: /core/resources/version.dat
    +        required: true
           - source: /bin/tmp/release/bearsampp.exe
             dest: /bearsampp.exe
    +        required: true
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: Adding required: true validation is a good practice to fail early if critical source files are missing, preventing incomplete or failed synchronizations.

    Medium
    Possible issue
    Add sync job error handling

    Add error handling and retry mechanism for sync job to handle temporary network
    issues.

    .github/workflows/sync.yml [7-10]

     jobs:
       sync:
         runs-on: ubuntu-latest
         steps:
    +    continue-on-error: true
    +    retry:
    +      max-attempts: 3
    +      wait-seconds: 30
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    __

    Why: Adding retry logic for the sync job would improve reliability by handling temporary network issues, though the syntax in the improved code is not entirely correct for GitHub Actions.

    Low
    • More

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant