basicallydan / forkability

:fork_and_knife: A linter for your repository.
https://basicallydan.github.io/forkability
MIT License
103 stars 19 forks source link

JavaScript projects #61

Open M-Zuber opened 9 years ago

M-Zuber commented 9 years ago

We currently treat projects written in JavaScript as NodeJs based projects. That does not cover all the cases. One example being front end projects that rely on Bower.

The way I see things, we should just change node to be javascript and lump everything together.

So that leaves us with two things:

basicallydan commented 9 years ago

I agree.

As much as I dislike bower too I guess it is still quite popular. What would you say to a requirement for either bower or NPM in a JS project? And if they have both use rules for both. On 16 Nov 2015 18:56, "Mordechai Zuber" notifications@github.com wrote:

We currently treat projects written in JavaScript as NodeJs based projects. That does not cover all the cases. One example being front end projects that rely on Bower.

The way I see things, we should just change node to be javascript and lump everything together.

So that leaves us with two things:

  • rename node to javascript
  • ads rules related to Bower, ect

— Reply to this email directly or view it on GitHub https://github.com/basicallydan/forkability/issues/61.

M-Zuber commented 9 years ago

I am not quite sure what you mean

basicallydan commented 9 years ago

So, let's say we have a "JavaScript" based project to replace "NodeJS". At the moment the requirement is a package.json file, for a "NodeJS" project but we could change the requirement to be either package.json or bower.json. See what I mean?

basicallydan commented 9 years ago

Ah but there's more...

If they have a package.json we can start applying rules like "No node_modules folder" which would not apply if bower.json is there instead. Conversely, if they have a bower.json file, then "No bower_components folder" could be a requirement.

This would all mean introducing two new concepts:

M-Zuber commented 9 years ago

And if the project has both? Or uses something like aurelia?

I think I've opened up pandoras box :scream: :cold_sweat: :astonished: Removed help-wanted label until stuff is clarified.

M-Zuber commented 9 years ago

And if the project has both?

This one at least is someone easy. It doesn't require OR-based rules. It should be enough to have conditional rules - in which case it should just work seamlessly even if a project only has one.

ex. for conditional rules:

module.exports = {
    name: 'Node JS',
    files: {
        'package.json file':{
                        path:/^package\.json/i,
                        dependant-rules:['No node_modules folder']
                 }
        'No node_modules folder': {
            path: /^node_modules/i,
            shouldExist: false,
            type:'tree'
        }
    }
};

Don't have any great ideas on how to prevent the dependent rules from running - maybe store them in another object other then files?

basicallydan commented 9 years ago

Hahah! No it's all godo! I think we just want to support both at the same time. If it's a node and bower project (as disgusting as that is to me), then requirements for both should be respected.

As for your suggestion on the structure... we could either embed the rule within the rule like:

module.exports = {
    name: 'Node JS',
    files: {
        'package.json file': {
            path: /^package\.json/i,
            'dependent-rules': {
                'No node_modules folder': {
                    path: /^node_modules/i,
                    shouldExist: false,
                    type: 'tree'
                }
            }
        }
    }
};

But this could easily get out of hand if we start putting dependents of dependents. Easier to build, but harder to maintain.

basicallydan commented 8 years ago

In a follow up to my one-way gitter convo:

  1. In order to find out the version tags we need to know the NPM package name
  2. This requires downloading package.json
  3. Downloading package.json is a domain-specific task for JS projects only, so it should be dependent on existence of package.json
  4. That sounds a lot like #61!
  5. Therefore we should get #61 done first
  6. Also it requires a bit more thought. I'll put further thoughts in #61

@M-Zuber suggested this as the syntax:

module.exports = {
    name: 'JavaScript',
    rules: {
        'No node_modules folder': {
            path: /^node_modules/i,
            shouldExist: false,
            type: 'tree'
        }
    },
    files: {
        'package.json file': {
            path: /^package\.json/i,
            'dependant-rules': ['No node_modules folder']
        }
    }
};

The naming of dependent-rules is IMO not quite right, but that's not so important. What is important is that for #30, we need to have special behaviour for projects that have package.json files (i.e. JS projects) WRT tags. So, the lintTags step would probably want to be involved in this, somehow.

In a similar way to how lintFiles uses separate files for each language (which it is not aware of), we could create functions that need to be run after lintTags which uses the results of lintTags to run a test. This should be discussed in more detail in #30 but I feel that the design of this issue's implementation needs to take this into consideration.

Therefore what I'd propose is something like this for langs/nodejs.js

module.exports = {
    name: 'JavaScript',
    rules: {
        'No node_modules folder': {
            path: /^node_modules/i,
            shouldExist: false,
            type: 'tree'
        },
        'npm tags match git tags': function (tags, next) {
            // Does the logic then executes next, which is a function
        }
    },
    files: {
        'package.json file': {
            path: /^package\.json/i,
            dependentRules: {
                files : ['No node_modules folder'],
                tags : ['npm tags match git tags']
            }
        }
    }
};

Of course, as I said, we can discuss the logic behind how tags-related functions works in more detail in #30 but for now, just to make it clear that the 'No node_modules folder' is a file-related rule is perhaps a good idea for futureproofing.

@M-Zuber Whaddya think?! Am I overcomplicating it? I am convinced we need to make these distinctions - whether I'm suggesting a good way to do it is something I am not sure of.

M-Zuber commented 8 years ago

Overall your proposal seems quite fine. I'm understanding it as the following :