Add documentation on migrating node.js #2256

yucheng11122017 · 2023-04-02T03:04:57Z

What is the purpose of this pull request?

In response to comment in #2233

Overview of changes:
Added page on devGuide on how to migrate Node.js

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Add documentation on migrating node.js

Checklist: ☑️

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

jovyntls

The guide looks comprehensive to me! Thank you @yucheng11122017 🎉

Minor nits: can we standardise the capitalisation of keywords like npm, node?

docs/devGuide/development/migratingNodeJs.md

yucheng11122017 · 2023-04-02T04:54:35Z

The guide looks comprehensive to me! Thank you @yucheng11122017 🎉

Minor nits: can we standardise the capitalisation of keywords like npm, node?

Thanks for the review @jovyntls! I fixed the nits accordingly. The indentation of the box is also now inline with the bullet points.

I changed all npm to 'npm' and node to 'Node.js'. Hope thats ok!

EltonGohJH

LGTM

jovyntls

Thanks for the fixes @yucheng11122017! Is it possible to not have this newline between a numbered point and its sub points?

yucheng11122017 · 2023-04-04T08:21:42Z

Thanks for the fixes @yucheng11122017! Is it possible to not have this newline between a numbered point and its sub points?

I tried to remove that line but it seems that this is margin that is automatically added in the bullets. I tried to follow these two posts (Link 1 and Link 2) but doesn't work.

Would it be better if I added spacing between the sub headings similar to Migrating to TypeScript?

damithc · 2023-04-04T08:40:41Z

I tried to remove that line but it seems that this is margin that is automatically added in the bullets. I tried to follow these two posts (Link 1 and Link 2) but doesn't work.

Yes, this is a MarkBind quirk I encounter often when writing bullet lists.

jovyntls · 2023-04-05T02:00:51Z

Using four spaces per indent should fix this issue! Leaving a minimal reproduction here in case this is useful for @damithc future uses:

### Migration steps 

1. first numbered point
    - xxx (this has two indentation levels instead of one)
    - xxx
1. second numbered point
    - xxx
    - xxx
1. third numbered point

output:

Maybe we should document this somewhere 🤔

As for this specific example, it works using four space indentations per bullet point! The <box> tags complicate things a little but after playing around with the spacing this seems to work for me:

### Migration steps 

1. Refactor any deprecated syntax
    - Go to the [Node.js changelog](https://nodejs.org/en/blog/release) of the new version
    - Go through list of deprecated syntax and check if it is being used in Markbind
    - Replace any deprecated syntax
2. Check that all user-facing functionalities are working 
    - A quick way to do this is to go to the [Reader Facing Features in the User Guide]({{baseUrl}}/userGuide/readerFacingFeatures.html) and check that these features are still working as expected.
3. Check that there are no issues with development setup 
    - Set up the development environment by running through the steps in [Setting Up]({{baseUrl}}/devdocs/devGuide/development/settingUp.html) to ensure there are no problems
4. Update GitHubActions 
    - Go to [Markbind/markbind-action](https://github.com/MarkBind/markbind-action) and update the Node.js version numbers
        - See [Update node version from 14 to 16 PR](https://github.com/MarkBind/markbind-action/pull/8/files) for an example
    - Test there are no issues with workflows
        - Testing instructions located here: [markbind-action]({{baseUrl}}/devdocs/devGuide/githubActions/markbindAction.html) and [markbind-reusable-workflows]({{baseUrl}}/devdocs/devGuide/githubActions/markbindReusableWorkflows.html) 
        <box type="info" seamless header="If a different npm version is needed"> 
        Install correct version of npm in `action.yml` and `fork-build.yml`. Refer to [Update node version from 14 to 16 PR](https://github.com/MarkBind/markbind-action/pull/8/files) to see where npm install should be run. </box>
5. Check deployment to Netlify/other platforms
    - Deployment to Netlify 
        - Follow steps in [Deploying to Netlify]({{baseUrl}}/userGuide/deployingTheSite.html#deploying-to-netlify) but change the `NODE_VERSION` value accordingly. Check there are no issues with deployment and deployed site is as expected.
        - Markbind has two repos [init-minimal-netlify](https://github.com/MarkBind/init-minimal-netlify) and [init-typical-netlify](https://github.com/MarkBind/init-typical-netlify) which allows deployment to Netlify by using a config file. Update the config file `netlify.toml` with the correct Node.js version and check that deployment using button in `README` works as expected.
        <box type="info" seamless header="If a different npm version is needed">
        To specify the npm version add an environment variable `NPM_VERSION` with the correct version number. </box>
    - Deployment to Github pages
        - Using the `markbind deploy command`
            - Build site using correct Node.js version 
            - Follow steps in [Using the `markbind deploy` command]({{baseUrl}}/userGuide/deployingTheSite.html#deploying-to-github-pages) and check there are no issues with deployment or with the site. 
        - Using CI platforms
            - Follow steps in [Using CI Platforms]({{baseUrl}}/userGuide/deployingTheSite.html#using-ci-platforms) but update the config files for the various CI Platforms to use the correct Node.js version. Try deploying and ensure there are no problems with deployment. 
            <box type="info" seamless header="If a different npm version is needed">
            Install the correct npm version before running npm commands. </box>
6. Update documentation
    - Update Node.js and npm version in documentation. See [Update to use Node 16](https://github.com/MarkBind/markbind/pull/2233/files#diff-0f8e38868f41667abec6adacbb5131fbd6999c4913fc43e3429390b744f7a1f3) as an example. <box type="tip" seamless>
Don't forget to update the version numbers in the example config files in [Deploying the Site]({{baseUrl}}/userGuide/deployingTheSite.html)!
</box>

jovyntls

A few minor consistency nits (and a clarification)!

jovyntls · 2023-04-05T02:03:47Z

docs/devGuide/development/migratingNodeJs.md

+<div class="lead">
+
+Node.js versions have to upgraded periodically before the version being used reaches its [end of life](https://endoflife.date/nodejs).
+This page outlines the steps of migrating to a higher version of Node.js


Suggested change

This page outlines the steps of migrating to a higher version of Node.js

This page outlines the steps of migrating to a higher version of Node.js.

jovyntls · 2023-04-05T02:04:52Z

docs/devGuide/development/migratingNodeJs.md

+
+Read more about Node.js release lines [here](https://nodesource.com/blog/understanding-how-node-js-release-lines-work/).
+
+npm version will be upgraded accordingly to the Node.js version. Hence, do check that this upgrade does not cause any issues.


Just checking, what does "accordingly to" refer to? Is it automatically upgraded or should I take any action?

Suggested change

npm version will be upgraded accordingly to the Node.js version. Hence, do check that this upgrade does not cause any issues.

The npm version will be upgraded accordingly to the Node.js version. Hence, do check that this upgrade does not cause any issues.

It should be upgraded automatically! I added further clarification in
7eff420

jovyntls · 2023-04-05T02:06:36Z

docs/devGuide/development/migratingNodeJs.md

+
+1. Refactor any deprecated syntax
+   - Go to the [Node.js changelog](https://nodejs.org/en/blog/release) of the new version
+   - Go through list of deprecated syntax and check if it is being used in Markbind


Suggested change

- Go through list of deprecated syntax and check if it is being used in Markbind

- Go through the list of deprecated syntax and check if it is being used in MarkBind

Let's standardise to use "MarkBind" (instead of "Markbind") throughout! Same for the other occurrences in this file :)

damithc · 2023-04-05T05:34:09Z

Using four spaces per indent should fix this issue! Leaving a minimal reproduction here in case this is useful for @damithc future uses:

@jovyntls Thanks for the tip. I tried it and it looks like a blank line anywhere in the middle the list causes a blank line to appear below every bullet point.

1. first point
   - some point
   - another point
1. second point
1. third point

--------------------

1. first point
    - some point
    - another point

1. second point
1. third point

yucheng11122017 · 2023-04-05T07:02:52Z

Thanks for helping fix the indentation problems @jovyntls! I think it didn't work because of what @damithc mentioned

Using four spaces per indent should fix this issue! Leaving a minimal reproduction here in case this is useful for @damithc future uses:

@jovyntls Thanks for the tip. I tried it and it looks like a blank line anywhere in the middle the list causes a blank line to appear below every bullet point.

Also fixed the nits in the documentation

jovyntls

LGTM!

Add documentation on migrating node.js

1232d91

jovyntls suggested changes Apr 2, 2023

View reviewed changes

docs/devGuide/development/migratingNodeJs.md Outdated Show resolved Hide resolved

docs/devGuide/development/migratingNodeJs.md Outdated Show resolved Hide resolved

docs/devGuide/development/migratingNodeJs.md Outdated Show resolved Hide resolved

yucheng11122017 added 2 commits April 2, 2023 12:31

Fix nits in documentation

02299b9

Standardise capitalization of Node and npm

033e0eb

EltonGohJH approved these changes Apr 3, 2023

View reviewed changes

jovyntls suggested changes Apr 4, 2023

View reviewed changes

jovyntls suggested changes Apr 5, 2023

View reviewed changes

yucheng11122017 added 3 commits April 5, 2023 14:12

Fix indentation of bullet

9632e38

Fix nits in documentation

21fbcfc

Add clarification on how npm version is affected by Node version

7eff420

jovyntls approved these changes Apr 5, 2023

View reviewed changes

yucheng11122017 merged commit 9ae7698 into MarkBind:master Apr 5, 2023

tlylt mentioned this pull request Apr 9, 2023

Broken links introduced in dev docs #2273

Closed

jovyntls mentioned this pull request Apr 15, 2023

Add note on netlify config when migrating Node.js #2246

Closed

1 task

yucheng11122017 mentioned this pull request Jun 25, 2023

"markdown-it-attrs limitation: New line attributes not correctly assigned to list items" #2313

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation on migrating node.js #2256

Add documentation on migrating node.js #2256

yucheng11122017 commented Apr 2, 2023

jovyntls left a comment

yucheng11122017 commented Apr 2, 2023

EltonGohJH left a comment

jovyntls left a comment •

edited

Loading

yucheng11122017 commented Apr 4, 2023 •

edited

Loading

damithc commented Apr 4, 2023

jovyntls commented Apr 5, 2023 •

edited

Loading

jovyntls left a comment •

edited

Loading

jovyntls Apr 5, 2023

jovyntls Apr 5, 2023

yucheng11122017 Apr 5, 2023

jovyntls Apr 5, 2023

damithc commented Apr 5, 2023 •

edited

Loading

yucheng11122017 commented Apr 5, 2023

jovyntls left a comment

	This page outlines the steps of migrating to a higher version of Node.js
	This page outlines the steps of migrating to a higher version of Node.js.


		Read more about Node.js release lines [here](https://nodesource.com/blog/understanding-how-node-js-release-lines-work/).

		npm version will be upgraded accordingly to the Node.js version. Hence, do check that this upgrade does not cause any issues.

	npm version will be upgraded accordingly to the Node.js version. Hence, do check that this upgrade does not cause any issues.
	The npm version will be upgraded accordingly to the Node.js version. Hence, do check that this upgrade does not cause any issues.

	- Go through list of deprecated syntax and check if it is being used in Markbind
	- Go through the list of deprecated syntax and check if it is being used in MarkBind

Add documentation on migrating node.js #2256

Add documentation on migrating node.js #2256

Conversation

yucheng11122017 commented Apr 2, 2023

jovyntls left a comment

Choose a reason for hiding this comment

yucheng11122017 commented Apr 2, 2023

EltonGohJH left a comment

Choose a reason for hiding this comment

jovyntls left a comment • edited Loading

Choose a reason for hiding this comment

yucheng11122017 commented Apr 4, 2023 • edited Loading

damithc commented Apr 4, 2023

jovyntls commented Apr 5, 2023 • edited Loading

jovyntls left a comment • edited Loading

Choose a reason for hiding this comment

jovyntls Apr 5, 2023

Choose a reason for hiding this comment

jovyntls Apr 5, 2023

Choose a reason for hiding this comment

yucheng11122017 Apr 5, 2023

Choose a reason for hiding this comment

jovyntls Apr 5, 2023

Choose a reason for hiding this comment

damithc commented Apr 5, 2023 • edited Loading

yucheng11122017 commented Apr 5, 2023

jovyntls left a comment

Choose a reason for hiding this comment

jovyntls left a comment •

edited

Loading

yucheng11122017 commented Apr 4, 2023 •

edited

Loading

jovyntls commented Apr 5, 2023 •

edited

Loading

jovyntls left a comment •

edited

Loading

damithc commented Apr 5, 2023 •

edited

Loading