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

Use readthedocs-common to share linting files accross different repos #3808

Merged
merged 5 commits into from Mar 23, 2018

Conversation

@humitos
Copy link
Member

@humitos humitos commented Mar 15, 2018

@humitos
Copy link
Member Author

@humitos humitos commented Mar 15, 2018

One minor problem that I see here is that modifying the config files on that repo won't affect inmmediately to the other repos (won't trigger a tox process with the lint env) and we won't know if that change is correct.

So, I think we could configure to trigger this once a PR is created in the readthedocs-common repo.

@agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Mar 15, 2018

I think that is actually a feature of using submodules here. It also allows us to pin to a specific commit in the common branch for each repo, so sudden changes won't automatically affect all repositories.

Hopefully we have things tuned enough now that it won't be too much of a problem though.

The one problem I anticipate with the shared repo is that changes to the common rules will require us to run checks against all of the parent repositories to ensure things don't break horribly. We'll work that out later though

- settings
- tastyapi
- templates
- vcs_support
Copy link
Contributor

@agjohnson agjohnson Mar 15, 2018

Hrm, this might be a file to leave out of the common repo for now. These includes are all specific to RTD

@@ -1,42 +0,0 @@
{
Copy link
Contributor

@agjohnson agjohnson Mar 15, 2018

I'll raise the issue in the common repo, but there probably isn't a reason to hide the files in the common repo. That is, a symlink would be:

ln -s readthedocs-common/eslintrc .eslintrc

Copy link
Contributor

@agjohnson agjohnson Mar 15, 2018

Copy link
Member

@ericholscher ericholscher left a comment

Looks good. I wonder if this will break random CI bits if they don't support submodules, but I think it's a good step forward.

@humitos humitos force-pushed the humitos/autolint/shared-configs branch from b3bbf79 to 4963979 Mar 22, 2018
@agjohnson agjohnson merged commit 3c674f0 into master Mar 23, 2018
1 check passed
@agjohnson agjohnson deleted the humitos/autolint/shared-configs branch Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants