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

Added eslint config and npm lint method #15

Merged
merged 6 commits into from
Mar 17, 2023
Merged

Added eslint config and npm lint method #15

merged 6 commits into from
Mar 17, 2023

Conversation

mefengl
Copy link
Contributor

@mefengl mefengl commented Mar 17, 2023

current code can't pass the npm run lint

.eslintrc.json Show resolved Hide resolved
@mefengl mefengl marked this pull request as ready for review March 17, 2023 09:02
@mefengl
Copy link
Contributor Author

mefengl commented Mar 17, 2023

@adamlui
it would be better to work on this branch to commit, you added the node_modules, why not put it into .gitignore.

.eslintrc.json Show resolved Hide resolved
.eslintrc.json Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

@adamlui it would be better to work on this branch to commit, you added the node_modules, why not put it into .gitignore.

Ok added

Cause .eslintrc.json is already here
@mefengl
Copy link
Contributor Author

mefengl commented Mar 17, 2023

IDK if you have any new comments cause there seems like some issue with GitHub conversation, I see the outdated label at the conversation, but can't get the newest conversation

so, copy & paste you comments directly in this pull request

@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

Ok I will use this to communicate. eslint failed beacuse of Error: 199:11 error 'module' is not defined no-undef. It is because it doesn't know this can be used in node.js, so it doesn't know what module is. Can you add this line to the top of chatgpt.js and then eslint will try again and should pass?

/* eslint-env browser,node */

@mefengl
Copy link
Contributor Author

mefengl commented Mar 17, 2023

it's because the eslint config is not set correctly, if we also want to use it in node, then should add a node in the eslint config file

@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

Great now all we need are whatever style rules for the GitHub action. Can you make it the default ones or is it already?

@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

it's because the eslint config is not set correctly, if we also want to use it in node, then should add a node in the eslint config file

Also I don't see this ever being usable in node because that's server side, but using npm means we can still publish to npmjs.com (where they accept front end libs too)

@mefengl
Copy link
Contributor Author

mefengl commented Mar 17, 2023

May use it as by npm install and then import it in another repo

@mefengl
Copy link
Contributor Author

mefengl commented Mar 17, 2023

The current GH action will use the .eslintrc.json we defined, so I guess it's ok

@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

Chatgpt told me to make the /.github/workflows/yml config file and that's where I inserted your rules, (and eslint check passed your last pr). Does it not depend on it anymore? If so we can remove that directory maybe

@mefengl
Copy link
Contributor Author

mefengl commented Mar 17, 2023

We always need that eslint.yml file to do ci/cd, that's how GH action works

@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

And that's where your rules are, how come you said 'The current GH action will use the .eslintrc.json we defined'

@adamlui
Copy link
Member

adamlui commented Mar 17, 2023

Oh never mind the style rules are not there

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.

CI/CD: format convention (eslint, GH action)
2 participants