Add Eslint to the built-in-report module

Development
1

I am hoping to add eslint to the built-in report module which helps to maintain the quality code. But there are two ways to add that.

  1. configure eslint with web pack and check the code style errors in the build time.
  2. Add code style check to the GitHub repo using hound ( ex: openmrs-owa-cohortbuilder it used this method)
  3. we can do both . :slight_smile:

So what method more preferable for the built in report module ?

2

What would you recommend? :smile:

3

I think we need to do both. Because of following reasons.

  1. if we choose 1st method since It will check code style errors in build time locally and a developer can mistakenly add code to main repo with errors.
  2. And we if we go for the 2nd method it will prevent the adding code to main repo with code style errors but it will check only when a developer tries to push the code. So developer can’t check code style error locally.

So My opinion is that do both things. :smile: But first I will configure with web pack. Is it ok?

4

I like your recommendation! So let us go ahead with it. :slight_smile:

1 Like
5

Ok, I will work on that. Thank you for your quick response @dkayiwa . :slight_smile:

6

For the eslint- rules I put rules used in openmrs-owa-cohortbuilder. I thought that same rules can be apply to built-in-report module.Any suggestions or modifications @judeniroshan, @dkayiwa?

7

Hi @ridmal,

Good to see your suggestions. I like that having flexibility over both the approaches is a good way to go. Because it will help us to define the strict validations when need. Github check is necessary and we can follow the owa-cohortbuilder which you have already referred. :slight_smile:

1 Like
8

When I am checking with existing code with eslint I have found this warning inside the router.jsx

Warning Component definition is missing display name, react/display-name

normally this warning occurs when we don’t put names to our components and router.jsx having a function without a name. So I modified router.jsx like this to prevent that warning.

export default function routes() {return ();}

Is it ok ? or any other suggestios?

9

I sent a PR for this task. Please review it and give your opinion :slight_smile:

1 Like
10

Appreciate your effort! :+1: Changes have merged!

1 Like
11

Thank you for your guidance.:slight_smile: