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

Implement a feature to enable user to run custom checks #1202

Open
ankita240796 opened this issue Jul 11, 2021 · 6 comments
Open

Implement a feature to enable user to run custom checks #1202

ankita240796 opened this issue Jul 11, 2021 · 6 comments
Labels
cat: rfc Issues that propose changes, or a Request For Comments. priority: low Issues and PRs that should be resolved, but can be postponed. status: wip Issues and PRs that are still a work in progress. type: feat Issues and PRs related to new features. version: minor Issues and PRs with new features belonging to the next minor release.

Comments

@ankita240796
Copy link

Implement a feature via which a user can specify path to a custom checks folder and run them.

@ankita240796
Copy link
Author

@Nytelife26 these are the design ideas I have come up with. Want to verify before proceeding:

Option 1

  • User can provide a path to a file that has custom checks. These checks are self-sufficient and do not depend on proselint tools. The file should contain a check method which will be run on input files.

Option 2

  • User provides a path to a custom folder that contains checks implemented in the same format as proselint/checks. These checks can use tools defined in the proselint folder.
  • While running checks, we copy over this sub-folder to proselint directory and run all checks within this folder. After the run is complete, we delete the copied over folder.

I feel option 2 is the better way to go here since it allows user to utilize the existing functionality. The only extra work here will be to validate the checks provided by user as input and ensure that they are in the same format as proselint checks.

@ankita240796
Copy link
Author

I can share a detailed design doc once we confirm the approach to take.

@Nytelife26
Copy link
Member

We had quite a lengthy discussion about this already.

In the end, @suchow and I came up with the idea to:

  • Host a directory of official extension checks that, while not universally applicable, may be useful to some and are maintained to a high standard under our ethos so people can download them if they so wish
  • Add a CLI argument to pass a local checks folder. Copying the local folder over would not be necessary, since proselint exports its tools as library functions, so local checks can still use them regardless of where they are located.

So, I think we should do everything to encourage putting people's work in the main repository and ensure the ones in there are well-vetted, good, high-quality extensions, but people are still allowed to point to their own local checks if they really want to or during development.

@Nytelife26 Nytelife26 added cat: rfc Issues that propose changes, or a Request For Comments. priority: low Issues and PRs that should be resolved, but can be postponed. status: wip Issues and PRs that are still a work in progress. version: minor Issues and PRs with new features belonging to the next minor release. type: feat Issues and PRs related to new features. labels Jul 12, 2021
@ankita240796
Copy link
Author

To confirm, the scope of this issue would be to fix the second part? Further, this might help as well: updating the readme to encourage people to submit their checks in the main repo: including instructions on creating a new check and explaining the significance of the test before submitting it to the main repo.

@ankita240796
Copy link
Author

Add a CLI argument to pass a local checks folder. Copying the local folder over would not be necessary, since proselint exports its tools as library functions, so local checks can still use them regardless of where they are located.

What about the config in this case? Should we expect the user to provide an updated config where all custom checks are also listed or should it be assumed that all custom checks are to be run even if not mentioned in config?

I am in favour of an updated config - just to be consistent with how the existing checks are run.

@Nytelife26
Copy link
Member

Nytelife26 commented Jul 18, 2021

To confirm, the scope of this issue would be to fix the second part?

What do you mean by that?

updating the readme to encourage people to submit their checks in the main repo: including instructions on creating a new check and explaining the significance of the test before submitting it to the main repo.

Perfect suggestion, that was part of what we were planning :) I can draft up the README changes, but we first need more consensus on the plan here.

What about the config in this case? Should we expect the user to provide an updated config [...] or should it be assumed that all custom checks are to be run even if not mentioned in config?

Configuration should never make erroneous assumptions - if they want to enable these additional checks every time, they must be specified in config. Else, we can only rely on CLI args for individual sessions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat: rfc Issues that propose changes, or a Request For Comments. priority: low Issues and PRs that should be resolved, but can be postponed. status: wip Issues and PRs that are still a work in progress. type: feat Issues and PRs related to new features. version: minor Issues and PRs with new features belonging to the next minor release.
Projects
None yet
Development

No branches or pull requests

2 participants