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.

Sign up for GitHub

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

Jump to bottom

Do some chores #132

Merged
merged 4 commits into from
Jun 13, 2022
Merged

Do some chores #132

merged 4 commits into from
Jun 13, 2022

Conversation

hanahmily
Copy link
Contributor

  • Introduce more strict checkers
  • Replace yarn with npm
  • Add node version checker
  • Bump up skywalking-eye

Signed-off-by: Gao Hongtao hanahmily@gmail.com

@hanahmily hanahmily added this to the 0.2.0 milestone Jun 13, 2022
@wu-sheng
Copy link
Member

Please fix CI.

@lujiajing1126
Copy link
Contributor

lujiajing1126 commented Jun 13, 2022 •
edited
Loading

Why we have to use npm over yarn?

lujiajing1126
lujiajing1126 reviewed Jun 13, 2022
View reviewed changes
ui/tools/checkversion.js Outdated

const fs = require("fs");

const nvmVersion = fs.readFileSync(".node-version").toString().trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose we can impose version check by setting the engine field in package.json

https://docs.npmjs.com/cli/v8/configuring-npm/package-json#engines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

engine is better to place the version. I updated the script

lujiajing1126
lujiajing1126 requested changes Jun 13, 2022
View reviewed changes
ui/.node-version Outdated Show resolved Hide resolved
@hanahmily
Copy link
Contributor Author

Why we have to use npm over yarn?

There are two motivations:

  • skywalking-eye opts to npm to generate license files.
  • UI contributors are using the element UI which prefers npm.

@lujiajing1126
Copy link
Contributor

Why we have to use npm over yarn?

There are two motivations:

  • skywalking-eye opts to npm to generate license files.
  • UI contributors are using the element UI which prefers npm.

Make sense. But I'm still confused with the .node-version file.

It there any toolkit which follows this dotfile?

@hanahmily
Do some chores
43b7dd3 
  * Introduce more strict checkers
  * Replace yarn with npm
  * Add node version checker
  * Bump up skywalking-eye

Signed-off-by: Gao Hongtao <hanahmily@gmail.com>
@hanahmily
Remove patch version from node version checking
8f87f7f 
Signed-off-by: Gao Hongtao <hanahmily@gmail.com>
@hanahmily
Using package.json to mark node version
9171aef 
Signed-off-by: Gao Hongtao <hanahmily@gmail.com>
@hanahmily
Install before checking version
bcdcd97 
Signed-off-by: Gao Hongtao <hanahmily@gmail.com>
@hanahmily
Copy link
Contributor Author

Why we have to use npm over yarn?

There are two motivations:

  • skywalking-eye opts to npm to generate license files.
  • UI contributors are using the element UI which prefers npm.

Make sense. But I'm still confused with the .node-version file.

It there any toolkit which follows this dotfile?

I removed this file.

@lujiajing1126 lujiajing1126 merged commit 808fc8f into main Jun 13, 2022
@lujiajing1126 lujiajing1126 deleted the chores branch June 13, 2022 09:35
@hanahmily hanahmily mentioned this pull request Jun 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lujiajing1126 lujiajing1126 lujiajing1126 approved these changes

@wu-sheng wu-sheng Awaiting requested review from wu-sheng

Assignees
No one assigned
Labels
chore
Projects
None yet
Milestone
0.2.0
3 participants
@hanahmily @wu-sheng @lujiajing1126