-
-
Notifications
You must be signed in to change notification settings - Fork 774
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
GitHub Actions: Implement ESlint #1442
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@averdin2 Can you let us know your progress in regards to this issue? Thank you =] Progress: |
Es Lint Getting Started: https://eslint.org/docs/user-guide/getting-started |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hi @ronaldpaek, thank you for taking up this issue! Hfla appreciates you :) Do let fellow developers know about your:- You're awesome! P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :) |
i. Friday - Sunday |
@macho-catt Hi, I'm looking at the previous SCSS linter rules and only selected a few sets of rules. Usually, I opt for the standard config rule set, which is usually recommended and will ensure more consistency. And I reviewed the earlier comments to figure out what rules to apply. I know I usually apply eslint:recommended, and know that will apply pretty good robust setup rules for my js files without enforcing any of the stricter styling rules like semicolons, training commas, etc... Should I set each rule individually and the least amount of rules applied? I considered applying eslint:recommended by itself or with eslint:standard. I also have another concern/suggestion as well. I know some of the js files I've checked also contain liquid syntax in them, which is why I currently get so many lint warnings. I checked the other day and set the file to be recognized as a liquid file, and the errors went away, but then we lost the IntelliSense, so that's not helpful plus, we want the linter to catch errors. And I believe even if we get eslint to work for JS stuff since the liquid syntax is in the js files, I think it would still throw an error and complain, and we would have to tell es-lint to ignore this block of code manually, like the example shown below. /* eslint-disable */
{% assign name = 'world' %}
/* eslint-enable */
console.log('Hello, ' + name + '!'); I think this solution would be okay for the first step, and then, we can separate the liquid portion, the data, and the js portion, the logic. |
Lastly, my last task was to fix the liquid syntax, and actually, that was kind of the reason why I took on this task, but I guess all of this stuff is tied together. I'm not that detailed and organized, so I took a lot of videos and pictures and am reporting my findings. I could not solve this issue yet, but I am thinking of reorganizing the liquid data, probably in a separate file. I'm just not 100% confident about how the data flows and is used, yet I still need to review the codebase some more. Oh yes, I actually ended up creating a lot of config files. I wanted to mention I had to init the root folder to node so I could install things on the package.json. I know I don't think I didn't it when I just did the linter for JS, but when I was looking into the pre-commit stuff since it's a front-end/client side thing, you need a package.json to be in the root, but I'm just reporting my findings I'm sure there is a better way, but at least we have some data, that this is all possible, and it can be done incrementally without breaking everything. Okay, I am done with my report. 😄 |
@ronaldpaek why did you unassign? Where should this issue be moved? |
Once this issue comes out of the icebox, This issue will need a really good read, and then a decision on what to do next by the dev lead. |
|
Dependency
#1441Overview
We need to implement a linter for our javascript code to standardize and maintain the code properly.
Action Items
Resources/Instructions
Additional resources:
The text was updated successfully, but these errors were encountered: