Plugin Code Linting

Development
1

I think we should start using the standard discourse code formatting for our plugin ecosystem, both for server side and client side code.

How to format whole of the plugin’s code quickly?

From your core discourse directory, run the following commands:

Client Side:

List all eslint related issues

yarn eslint --ext .js,.js.es6 plugins/discourse-ratings/assets/javascripts

Fix all eslint related issues

yarn eslint --ext .js,.js.es6 --fix plugins/discourse-ratings/assets/javascripts

List all the linting issues in client js code directories

yarn prettier --check "plugins/discourse-ratings/{assets, config}/**"

List all the js files that are not formatted correctly

yarn prettier --list-different "plugins/discourse-ratings/{assets, config}/**"

Fix all the linting issues with js files in your assets and config folders.

yarn prettier --write "plugins/discourse-ratings/{assets, config}/**"

Note: assets and config folder are the only valid targets for client side code linting in case of discourse.

List all the issues with your handlesbars(.hbs only) code

yarn ember-template-lint --no-ignore-pattern plugins/discourse-ratings
You can use the --verbose flag which will print the code which has issues in the console.

Server Side

List all the linting issues in server code

bundle exec rubocop plugins/discourse-ratings/

List target files in a path

bundle exec rubocop -L plugins/discourse-ratings/

Fix all the issues (only safe)

bundle exec rubocop -a plugins/discourse-ratings/

Fix all the issues (safe and unsafe)

bundle exec rubocop -A plugins/discourse-ratings/

Note: the unsafe option will replace the code which is equivalent but not semantically similar. The best course of action would be to run tests after using the unsafe option or use your judgement. More on autocorrect here

Note: using bundle exec ensures that rubocop is being run with discourse project’s config

General Workflow Guidelines

  • Always commit your code before running these tools especially when running on bigger chunks of code or multiple files, so that you can go back to the actual state.
2
3

@developers

Thoughts?

4

@fzngagan I like it. SImple to do and will improve our workflow. @merefield thoughts?

5

@developers

I’ve added a few more points about code linting. If any of us has questions about any of these or wants to know more, I can find the answers for those queries too.

I’ve also created two PRs to demonstrate how the changes look after running the linting commands.

https://github.com/paviliondev/discourse-ratings/pull/33

https://github.com/paviliondev/discourse-ratings/pull/32

Also, once we get past this step, the formatting thing will be a loooot less daunting that this one :wink: .

6

Look good. Thanks @fzngagan!

7

@developers
A gentle reminder. Please try and run the commands in the OP before our call tomorrow.

Additionally, I’ve added a command to list issues with .hbs templates too. Although, we can keep that for the next week. Here’s a PR demonstrating the template linting issues and their fixes.

https://github.com/paviliondev/discourse-ratings/pull/35

8

Ok I did this for the custom wizard plugin

Client

I got a ton of warnings for when doing the dry runs. The real run produced this result

https://github.com/paviliondev/discourse-custom-wizard/commit/6f977049fc231146096449ed46f94ba6f1ef37b8

A few notes:

  1. Are we sure this is exactly the same prettier formatting that discourse/discourse uses? For example are they using trailing commas at the end of method declarations? I want to be 100% sure.

  2. Are we sure we have the right formatting for scss files as well?

When I run the template linting command, I see a long list of errors but no files are changed.

Server

The real run (safe only) produced this result

https://github.com/paviliondev/discourse-custom-wizard/commit/484bd464d2f7c4b6f1cf6a072aea87de7939eccc

It’s pretty much only adding and removing spaces. No harm there I think.

I’d like to run the unsafe one entirely seperately.

9

I’ll respond to the concerns during the call :slight_smile:

Also, a few updates to share this week too.

10

@developers

I’ve added the eslint commands too. Just a reminder that we want to get the prettier and rubocop formatting in place before our call. :slight_smile:

11

  • Can we make sure the es linting command is restricted to non-hbs files as that just adds to noise or add an extension to deal with them?, e.g.:
    image

  • Is it possible to auto-lint the hbs files instead of just getting a list of issues? Running on discourse-locations I get a lot of errors, but few stated as fixable?:
    image

12

I’ve shared the command with restricted file extensions. Note --ext flag.

Most of the things are not fixable with the template linter unfortunately. But if you use --fix it will help with some issues.

13

2 out of 100 :sweat:

14

Ah, could you update the earlier command?:

yarn prettier --check "plugins/discourse-ratings/{assets, config}/**"

15

Oh you mean with prettier? I’ll check that now.

16

yarn eslint --ext .js,.js.es6 plugins/discourse-locations/assets/javascripts

image

:sweat:

yarn ember-template-lint --no-ignore-pattern plugins/discourse-locations

image

:cold_sweat:

bundle exec rubocop plugins/discourse-locations/

image

:sweat_smile:

17

for prettier, There’s no CLI way of ignoring or specifying file extensions. I think you can ignore the noise and use the --fix option. You need to look for the following text at the bottom. :wink:

Screenshot 2021-03-10 at 1.15.03 PM

18
19

How should one run these tests within a dev docker environment?

OH!

Seems I’ve found out by accident.

First you have to run …

d/shell

then just

yarn

which appears to install everything.

Finally you can run your lint command.

20

@Angus, we should move the OP (at least) to somewhere public, this is such a useful topic?

PS done.