Skip to content

Update markbind deploy success log message#2852

Merged
yihao03 merged 12 commits intoMarkBind:masterfrom
yihao03:feat/update-deploy-log
Mar 25, 2026
Merged

Update markbind deploy success log message#2852
yihao03 merged 12 commits intoMarkBind:masterfrom
yihao03:feat/update-deploy-log

Conversation

@yihao03
Copy link
Copy Markdown
Contributor

@yihao03 yihao03 commented Mar 2, 2026

  • Update deploy log message
  • Refactor helper function into util directory
  • Lint and update tests

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Addresses #2849.
Now update the log message to point to URL of the GitHub Actions and the deployed site URL

Anything you'd like to highlight/discuss:

Testing instructions:
Deploy a website using markbind and observe the log message.

Proposed commit message: (wrap lines at 72 characters)
Update deploy log message to include GitHub Actions URL and deployed site URL


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 96.96970% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 70.41%. Comparing base (510231a) to head (d28819e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
packages/core/src/Site/SiteDeployManager.ts 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2852      +/-   ##
==========================================
+ Coverage   70.29%   70.41%   +0.11%     
==========================================
  Files         129      129              
  Lines        6951     6952       +1     
  Branches     1573     1543      -30     
==========================================
+ Hits         4886     4895       +9     
+ Misses       2059     1957     -102     
- Partials        6      100      +94     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

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

Updates MarkBind’s deploy command output to be less misleading by reporting both the GitHub Actions workflow status URL and the expected deployed site URL, while refactoring the CLI logging into a utility and expanding unit tests around URL construction.

Changes:

  • Extend deploy URL generation to return both GitHub Pages and GitHub Actions URLs.
  • Add a CLI utility to log deploy results in a clearer, less “success-looking” way.
  • Add unit tests for GitHub remote parsing and URL construction helpers.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/core/src/Site/SiteDeployManager.ts Introduces DeployResult and URL parsing/construction helpers; returns both GH Pages + Actions URLs from deployment URL resolution.
packages/core/index.ts Re-exports DeployResult type for CLI consumption.
packages/cli/src/util/deploy.ts Adds logDeployResult helper to centralize deploy logging behavior.
packages/cli/src/cmd/deploy.ts Switches deploy command to use the new logging helper and updated deploy return shape.
packages/core/test/unit/Site/SiteDeployManager.test.ts Adds unit tests for parsing remotes and constructing deploy URLs (including CNAME behavior).

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

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown
Member

@gerteck gerteck left a comment

Choose a reason for hiding this comment

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

Wondering, does the changes account for enough flexibility in supporting deployment through other CI platforms (e.g. Travis, AppVeyor CI, Circle CI), and also deploying to
netlify?

https://markbind.org/userGuide/deployingTheSite.html#deploying-to-github-pages

Edit: Will take a look at this again

  • Disregard the previous comment above ^, I misunderstood as the deploy command is by default configured for GitHub pages, and not meant to be flexible for other CI platforms or deployments.

@gerteck
Copy link
Copy Markdown
Member

gerteck commented Mar 8, 2026

Todos:

  • Address copilot comments regarding some edge cases (either implement, or comment why not applicable and resolve them)
  • Directly put the function in the same file cmd/deploy.ts where it is called, don't need it in a separate util file as it's 15 lines and not used elsewhere

Otherwise great work on this 👍

@gerteck
Copy link
Copy Markdown
Member

gerteck commented Mar 18, 2026

Maybe can tackle #1575 at the same time in this PR, or close it if it already fixes it

@yihao03 yihao03 force-pushed the feat/update-deploy-log branch from 83f4ea7 to 1f6b153 Compare March 18, 2026 10:54
@MoshiMoshiMochi MoshiMoshiMochi self-requested a review March 19, 2026 11:12
Copy link
Copy Markdown
Member

@gerteck gerteck left a comment

Choose a reason for hiding this comment

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

lgtm, the export of DeployResult seems a bit weird but I suppose this is the best way to do it for now

Windows CI failing is due to separate issue.

Copy link
Copy Markdown
Contributor

@MoshiMoshiMochi MoshiMoshiMochi left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just seeking clarification on one small part regarding the support for enterprise urls.

Image

@yihao03 yihao03 merged commit d552775 into MarkBind:master Mar 25, 2026
10 checks passed
@github-actions github-actions bot added the r.Patch Version resolver: increment by 0.0.1 label Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

r.Patch Version resolver: increment by 0.0.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants