Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Default Config & Disable If No Project Config settings #101

Merged
merged 3 commits into from
Feb 16, 2018

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Jan 30, 2018

image

Adds settings to use a default config if no project config is found. The default setting is ~/.atom like other linters.

I also added a checkbox to disable coffeelint if no project config is found.

closes #98

@Arcanemagus
Copy link
Member

The default setting is ~/.atom like other linters.

What other provider does this? ~/.atom should be for Atom's settings not something like coffeelint.

I also added a checkbox to disable coffeelint if no project config is found.

Awesome! Reviewing this now.

@UziTech
Copy link
Member Author

UziTech commented Jan 30, 2018

What other provider does this? ~/.atom should be for Atom's settings not something like coffeelint.

Sorry, I thought linter-eslint defaulted to that directory but that is just what I have it set to. This would be
default configuration for coffeelint in Atom so it would make sense to keep it in ~/.atom.

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, just a few minor changes.

src/index.js Outdated
defaultConfig: {
title: 'Default Config Path',
type: 'string',
default: atom.getConfigDirPath(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just leave this blank, if a user wants to fill this in they can enter the path to wherever it should be themselves. The default value as is is wrong and just encourages people to put non-Atom configuration files in there.

src/index.js Outdated
@@ -17,6 +17,19 @@ const loadDeps = () => {
};

module.exports = {
config: {
defaultConfig: {
title: 'Default Config Path',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a description clarifying that this is only used if coffeelint can't find a configuration on it's own.

@UziTech
Copy link
Member Author

UziTech commented Jan 30, 2018

I updated it to look more like linter-eslint

@jhessin
Copy link

jhessin commented Feb 16, 2018

This sounds like a good update - can someone with access merge it?

@Arcanemagus
Copy link
Member

Whoops, this got lost in my notifications. @UziTech would you be interested in being added as a maintainer here?

@UziTech
Copy link
Member Author

UziTech commented Feb 16, 2018

That sounds great 😃👍

@Arcanemagus
Copy link
Member

Added, feel free to merge this whenever 😉. Please don't hesitate to contact me on Atom's Slack, as you can see sometimes it takes a while for me to get to a GitHub notification 😆.

@UziTech UziTech merged commit c8771fd into AtomLinter:master Feb 16, 2018
@UziTech
Copy link
Member Author

UziTech commented Feb 16, 2018

@Arcanemagus sounds good. Is there a process for pushing a new version to APM?

@Arcanemagus
Copy link
Member

You can update the changelog using github_changelog_generator (You'll need to add the configuration file (like this one and remove it from .gitignore) first, and then after committing that follow the steps in the Flight Manual.

Alternatively you can:

  1. Update the changelog
  2. Run npm version <foo> --no-git-tag
  3. Run a script like this

Since you aren't signing your commits, the first option is probably easier. Also, I just started a discussion in the team area about automating the entire process that needs some feedback 😉.

@UziTech UziTech deleted the disable-if-no-config branch February 17, 2018 06:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to disable when no rules are found.
3 participants