search

securesauce / precaution-beta

Precaution provides a simple, automated code review for GitHub projects by running code linters with a security focus on pull requests.
Other
1 stars 0 forks source link

Document our decision to ignore tool specific configuration files #218

Closed joshuagl closed 5 years ago

joshuagl commented 5 years ago

We are opting to ignore tool specific configuration files which may exist in the repository and only use configuration which has been set in precaution.json

This strategy allows us to expose relevant functionality common to all supported linters in a consistent fashion. Furthermore by not exposing tool specific configuration via Precaution we don't have to worry about migration paths should we choose to change the underlying tools in future.

Closes: #81

Signed-off-by: Joshua Lock jlock@vmware.com

MVrachev commented 5 years ago

Maybe it won't be a bad idea to update the architecture in this pr given that we are saying that we hide the complexity of the underlying linters?

joshuagl commented 5 years ago

Maybe it won't be a bad idea to update the architecture in this pr given that we are saying that we hide the complexity of the underlying linters?

We're explicitly hiding the complexity of the underlying linters from users, not developers. We should abstract the linters in the diagram as previously discussed, but I don't think it needs to happen in this PR?

MVrachev commented 5 years ago

Maybe it won't be a bad idea to update the architecture in this pr given that we are saying that we hide the complexity of the underlying linters?

We're explicitly hiding the complexity of the underlying linters from users, not developers. We should abstract the linters in the diagram as previously discussed, but I don't think it needs to happen in this PR?

Ookay, we can use another pr for that. I think that this text should be added in a future doc for the configuration options. Maybe we can keep it both places?

joshuagl commented 5 years ago

Agreed, I wanted to document the decision we made and prompt a GitHub Pages build by updating a file in docs. When we document our configuration we should include some user-focused text around this too.

joshuagl commented 5 years ago

Rebasing the branch to integrate the latest changes from the master branch dismissed the review.