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

JavaScript projects #61

Open
M-Zuber opened this issue Nov 16, 2015 · 9 comments
Open

JavaScript projects #61

M-Zuber opened this issue Nov 16, 2015 · 9 comments

Comments

@M-Zuber
Copy link
Collaborator

M-Zuber commented Nov 16, 2015

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
@basicallydan
Copy link
Owner

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" [email protected] 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
#61.

@M-Zuber
Copy link
Collaborator Author

M-Zuber commented Nov 17, 2015

I am not quite sure what you mean

@basicallydan
Copy link
Owner

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
Copy link
Owner

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:

  • OR-based rules
  • Conditional rules (which would be sort of sub-rules)

@M-Zuber
Copy link
Collaborator Author

M-Zuber commented Nov 22, 2015

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

I think I've opened up pandoras box 😱 😰 😲
Removed help-wanted label until stuff is clarified.

@M-Zuber
Copy link
Collaborator Author

M-Zuber commented Nov 22, 2015

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
Copy link
Owner

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
Copy link
Owner

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 JavaScript projects  #61!
  5. Therefore we should get JavaScript projects  #61 done first
  6. Also it requires a bit more thought. I'll put further thoughts in JavaScript projects  #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
Copy link
Collaborator Author

M-Zuber commented Nov 30, 2015

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

  • Run these rules
    • if there are dependent rules, run them
    • each dependant rule is in an array value where the key defines the relevant linter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants