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

[FIX] Fix deploy workflow #412

Merged
merged 2 commits into from
Dec 19, 2024
Merged

[FIX] Fix deploy workflow #412

merged 2 commits into from
Dec 19, 2024

Conversation

surchs
Copy link
Contributor

@surchs surchs commented Dec 19, 2024

Appzi and plausible script were not correctly inserted. This caused https://github.com/neurobagel/query-tool/actions/runs/12415438011/job/34661801769 to break

  • Closes #

Changes proposed in this pull request:

  • remove accidentally included script dangles in index.html
  • fix mistakes in deploy WF

NOTE: If this pull request is to be released, the release label must be applied once the review process is done to avoid the local and remote from going out of sync as a consequence of the bump version workflow run

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass
  • If the PR changes the participant-level and/or the dataset-level result file, the query-tool-results files in the neurobagel_examples repo have been regenerated

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Fix the deployment workflow to correctly insert the Appzi and Plausible scripts into the index.html file.

CI:

  • Correctly insert Appzi and plausible scripts into the production index.html file.

Deployment:

  • Fix the deployment workflow to correctly insert the Appzi and Plausible scripts.

Appzi and plausible script were not correctly inserted
Copy link

sourcery-ai bot commented Dec 19, 2024

Reviewer's Guide by Sourcery

This pull request fixes the deployment workflow by correctly inserting the Appzi and Plausible scripts into the index.html file. The fix involves combining the two script tags into a single line within the add_script variable. This corrected a formatting issue that prevented the scripts from being properly included in the deployed website.

Sequence diagram for the deployment workflow script insertion

sequenceDiagram
    participant D as Deploy Workflow
    participant H as index.html
    D->>H: Read file content
    Note over D: Prepare single-line script tags
    D->>H: Insert Appzi and Plausible scripts
    Note over H: Scripts properly embedded
Loading

File-Level Changes

Change Details Files
Correctly insert Appzi and Plausible scripts
  • Combined the two script tags within the add_script variable into a single line to ensure proper insertion into the index.html file during deployment.
  • Removed unnecessary newlines and indentation from the add_script variable definition to improve readability and maintainability.
.github/workflows/deploy.yaml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for neurobagel-query ready!

Name Link
🔨 Latest commit 8eef317
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-query/deploys/67644973c9e0920008bddff2
😎 Deploy Preview https://deploy-preview-412--neurobagel-query.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@surchs surchs requested a review from alyssadai December 19, 2024 15:25
- correct WF invocation of sed
- remove dangling appzi scripts in index.html
Copy link
Contributor

@alyssadai alyssadai left a comment

Choose a reason for hiding this comment

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

🧑‍🍳

@alyssadai
Copy link
Contributor

Don't forget a PR label @surchs!

@surchs surchs added the pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) label Dec 19, 2024
@surchs surchs merged commit cd50615 into main Dec 19, 2024
10 checks passed
@surchs surchs deleted the fix-appzi branch December 19, 2024 17:08
Copy link
Contributor

🚀 PR was released in v0.8.1 🚀

@neurobagel-bot neurobagel-bot bot added the released This issue/pull request has been released. label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) released This issue/pull request has been released.
Projects
Status: Review - Done
Development

Successfully merging this pull request may close these issues.

2 participants