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

Deploy portfolio on netlify #2593

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

IanCheah
Copy link

@IanCheah IanCheah commented Feb 5, 2025

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:

Fixes #2418

Overview of changes:

  • Deployed portfolio site on netlify
  • Updated the templates.md page

Anything you'd like to highlight/discuss:
The porfolio site is publicly accessible using the following url https://markbind-template-portfolio.netlify.app/.

Testing instructions:

  1. run the command markbind s
  2. navigate to the templates page
    i. Click on the USER GUIDE tab located at the top of the home page
    ii. Click on the Templates tab located at the side, under Working with Sites

Proposed commit message: (wrap lines at 72 characters)
Deploy portfolio on Netlify


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).

@lhw-1
Copy link
Contributor

lhw-1 commented Feb 7, 2025

The porfolio site is publicly accessible using the following url https://markbind-template-portfolio.netlify.app/.

Quick link here

@lhw-1
Copy link
Contributor

lhw-1 commented Feb 7, 2025

@IanCheah The deployment itself looks rather different from this one here mentioned in the PR #2398 for the template; there are of course bound to be some changes, but the icons etc. seem to be displayed incorrectly?

@gerteck
Copy link
Contributor

gerteck commented Feb 7, 2025

Reference Documentation Site in Question: https://markbind.org/userGuide/templates.html

Something doesn't seem right... The buttons represent deploy to Netlify links, not sample sites of the templates.

image

These are actually quick deploy links provided by MarkBind, that create a repository for you and deploy in a single click.

In fact, this highlights that the project template netlify-deploy button is also wrongly labelled.
It was probably overlooked in #2400 because a great deal of effort was spent in the design of the site, such that the documentation was wrongly labelled.

Also the site https://markbind-template-portfolio.netlify.app/ looks broken on my end too. (Needs to be investigated).

I have a few suggestions:

  • Add maybe a separate column as 'sample site' or 'example site' for each template.

  • Consider adding this for default and minimal as well.

  • Fix the project template quick deploy link, because it's not a quick deploy link. Instead, maybe put it under sample site or related, as it's quite helpful.

  • Fix and simplify the portfolio template, right now it seems a bit convoluted (and also seems broken). As discussed in our last meeting, it could be beneficial to simplify and even target certain audiences (i. e. cs2103t / cs2113 students) since they have a touchpoint with markbind and are probably the most likely to be new users. This could mean adding a sample tP project, and maybe an orbital section as well.

These suggestions could be made out into separate issues as well to be appropriate handled and tracked.

Cheers 👍

@Tim-Siu
Copy link
Contributor

Tim-Siu commented Feb 10, 2025

Reference Documentation Site in Question: https://markbind.org/userGuide/templates.html

Something doesn't seem right... The buttons represent deploy to Netlify links, not sample sites of the templates.

image

These are actually quick deploy links provided by MarkBind, that create a repository for you and deploy in a single click.

In fact, this highlights that the project template netlify-deploy button is also wrongly labelled. It was probably overlooked in #2400 because a great deal of effort was spent in the design of the site, such that the documentation was wrongly labelled.

Also the site https://markbind-template-portfolio.netlify.app/ looks broken on my end too. (Needs to be investigated).

I have a few suggestions:

  • Add maybe a separate column as 'sample site' or 'example site' for each template.
  • Consider adding this for default and minimal as well.
  • Fix the project template quick deploy link, because it's not a quick deploy link. Instead, maybe put it under sample site or related, as it's quite helpful.
  • Fix and simplify the portfolio template, right now it seems a bit convoluted (and also seems broken). As discussed in our last meeting, it could be beneficial to simplify and even target certain audiences (i. e. cs2103t / cs2113 students) since they have a touchpoint with markbind and are probably the most likely to be new users. This could mean adding a sample tP project, and maybe an orbital section as well.

These suggestions could be made out into separate issues as well to be appropriate handled and tracked.

Cheers 👍

Great catch on the misleading "Deploy to Netifly" label. I also agree that we should add a seperate column about sample deployment.👍

Copy link

codecov bot commented Feb 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.86%. Comparing base (92d2373) to head (a667c5f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2593   +/-   ##
=======================================
  Coverage   51.86%   51.86%           
=======================================
  Files         127      127           
  Lines        5474     5474           
  Branches     1201     1201           
=======================================
  Hits         2839     2839           
  Misses       2340     2340           
  Partials      295      295           

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

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.

Set up deployment for student portfolio template
4 participants