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

Projects
None yet
3 participants
@humitos
Member

humitos commented Mar 15, 2018

@humitos

This comment has been minimized.

Member

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

This comment has been minimized.

Contributor

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

This comment has been minimized.

@agjohnson

agjohnson Mar 15, 2018

Contributor

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 @@
{

This comment has been minimized.

@agjohnson

agjohnson Mar 15, 2018

Contributor

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

This comment has been minimized.

@agjohnson

@agjohnson agjohnson referenced this pull request Mar 15, 2018

Closed

Unhide files #1

@ericholscher

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.

@agjohnson agjohnson merged commit 3c674f0 into master Mar 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@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