Ideas for tests

[jimaek]

• files/$projectName should contain only 1 file. info.ini
• mainfile parameter should be correct. The file specified should actually exists.

I will add more later as more PRs come in and I see the mistakes :)

[jimaek]

Btw we should make sure the user understands the difference between Bot's suggestions and warnings/errors that need to be fixed. Suggestions can be ignored, warnings/errors not

[megawac]

Well lets start a list (this is a start) -- feel free to edit this @jimaek

• [x] Only info.ini under files/:version and it exists
• [x] proper quotes in info.ini
• [x] mainfile exists
• [x] version file structure is consistent
• [x] no file changes under files/:project/:version
• [x] file type is accepted
• [x] maximum 10 lines in a minified file
• [x] author name 2 - 30 characters long
• [x] info.ini fields ["author", "description", "mainfile"] exist and other known properties include ["github", "homepage"] are known
• [x] info.ini is entirely ascii
• [x] folder ~~and file names~~ are in lower case
• [ ] validate map files
• [ ] check relative paths
• [ ] check for non absolute amd dependencies?
• [x] check if extension matches it being a binary file
• [ ] check if malicious javascript (any plugin?)
• [ ] validate file extension to content type (any plugin?)
• [x] validate homepage and github website respond with a reasonable status
• [x] validate github page is indeed a github page
• [x] check for unicode files in files
• [x] throw hissy fit when a file is removed
• [x] ~~check if comment header given to js and css files~~
• [x] ~~check project already exists~~
• [x] validate update.json fields align with libgrabbers' spec
• [x] check changes between versions

[jimaek]

• Lowercase only for project names and not filenames.

[jimaek]

• 1 PR = 1 Project. Make sure the use is not trying to modify/update multiple projects at the same time

[tomByrer]

Add Minifed files only please. Easiest is to check for no newlines in the last 99-200 characters. (&/or no newlines after the top-line comment if the file is very short).

Lowercase only for project names and not filenames

I still believe that filenames should be lower-case, but I assume you'll still trump me on this ;)

[megawac]

• Verifying that a file is minified is tricky, your test will not work with jsmin (last line always a new line), yui-compress or closure compiler default settings when the previous line is full (something like 500 lines) and other considerations. If someone can provide a robust test for minified files, pr it.
• I agree with lower case file names.
• Don't agree with failing for providing non-minifed files, I'm for jimaek continuing to use his discretion

[tomByrer]

jsmin (last line always a new line)

Yes, that is a common EOF lately. Well skip the last 3 characters then :) I didn't know about Closure's 500-char limit; cheers! I don't know Python; will a RegEx be a good starting point?

Don't agree with failing for providing non-minifed files, I'm for jimaek continuing to use his discretion

I was thinking more of a soft-warning. Plus jimaek is telling people to at least add minified files, & starting to ask not including non-minifed files.

[jimaek]

"Minified files only" is applied only in a few cases:

• Its a new project
• The official repo contains both minified and unminified versions
• Other unpredicted situations

In all other cases we host both minified and unminified. So I dont think we need the bot checking for that. It will only add useless information that mods will ignore.

I am against lowecase filenames. Some examples why

• We should rehost what the author provides
• Some projects have dependencies inside them and if we rename all files they will fail so we would need to check the sources of all js/css files and rename the filenames there too.
• Some projects can have tens and hundreds of files so to convert all to lowercase it would require the user to have linux. This will turn away people that use windows/web and wanted to contribute.

[tomByrer]

Some projects have dependencies inside them

That's a good point.

to convert all to lowercase it would require the user to have linux

Where do you get this misinformation from please? https://www.google.com/search?q=windows+util+lowercase The reason why all filenames should be lower case is BECAUSE of windows ignoring uppercase, & *ux/OSX treating upper & lower case files as different files. There are repos that have to change over because they're forced to by users, Bower, NPM, etc. I just want to catch the messy transition over to lowercase early, like ReactJS had to.

[jimaek]

Where do you get this misinformation from please?

Im not saying its not possible. But a Windows user is not really used to working with a command line and batch files. There are gui soft for this too but it requires so many extra steps that it makes it too complicated for the user and he will lose all interest.

I know the win/unix difference but I really believe its 100x times easier to deal with the rare changes to lowercase by some projects than force all projects to be lowercase at all times.

[tomByrer]

There are gui soft for this too but it requires so many extra steps that it makes it too complicated for the user

Oh good grief; the repo owner is likely using git, lowercase is not more complicated than git for sure. If a non-repo-owner wants to volunteer, then they should at least know how to type.

We need to protect the actual users from idiots, not allowing idiots to make more hassles down the road with messy PRs. It's not like lowercase-only is alien: https://github.com/cdnjs/cdnjs#conventions That would be a mess in itself; jsDelivr having uppercase, when other CDNs have lower for the same files.

[jimaek]

cdnjs may have it in their readme but they don't follow it either http://cdnjs.com/libraries/jquery-contextmenu

Also when I am talking about users I always mean people that just wanted to help and not actual authors. I know an author wont have any issues to convert his files to lowercase :)

If we decided to force lowercase for files it would bring much more problems that it would solve. Dependencies in files, people using previous versions of a project with upercase...

Most projects using uppercase letters will never switch to lowercase so it will never be an issue. But if we forced it we would simply create problems for ourselves.

[megawac]

I see what you mean @jimaek but I'd argue that the server should evaluate the lowercase equivalent to the requested uri.

http://cdn.jsdelivr.net/tablesorter/2.16.4/js/widgets/widget-staticrow.js should be the same as http://cdn.jsdelivr.net/tablesorter/2.16.4/js/widgets/widget-staticRow.js in my opinion.

http://cdn.jsdelivr.net/g/tablesorter(js/widgets/widget-staticRow.js) == http://cdn.jsdelivr.net/g/tablesorter(js/widgets/widget-staticrow.js)

Then it would make sense to add a check for the bot to make sure there aren't duplicate files

[jimaek]

I am afraid there is no way to make Nginx case insensitive as it relies on the OS for that. If you have any ideas let me know.

[megawac]

@tomByrer a working regexp would be great, currently the check is something like file name contains \bmin\b and is less than 20 lines

@jimaek this thread is getting cluttered, I'm going to start deleting comments

[tomByrer]

http://cdn.jsdelivr.net/tablesorter/2.16.4/js/widgets/widget-staticrow.js should be the same as http://cdn.jsdelivr.net/tablesorter/2.16.4/js/widgets/widget-staticRow.js in my opinion.

For Unix-based OS, it will never never be. Trust me, I had to make a case script 20 years ago when I transferred files from my 8088 DOS computer to a Sun SPARKstation. Exactly why all-lower-case filenames are a de-facto standard for files that will cross OSes (eg web). Sooner or later, there will be problems; better fix them earlier IMHO.

[jimaek]

@tomByrer I hope I was able to convince you why its a bad idea to convert everything to lowercase :)

[tomByrer]

Your dependency issue is a good point. But that is what I'm trying to also prevent; people used to (become dependent on) mixed/upper cased files.

I know you don't like policing too much, but I think this worth spending time to invest in earlier.

[tomByrer]

New idea! If minified file has sourceMappingURL\s*=, then there should be a .map file. Rare issue, but does happen.

[jimaek]

@megawac I left a comment here https://gist.github.com/jsdelivrbot/3c17c9427c31abe15123

[megawac]

Validate source maps https://github.com/mattrobenolt/python-sourcemap

[tomByrer]

Validate source maps https://github.com/mattrobenolt/python-sourcemap

Neat idea, but you'll need the non-minified source files to compare against (which we're trying to prevent uploading) or have the uploader add tests.

Most complaints about source maps are they don't exist (we've almost solved that already) & they are not working is that they don't work as intended in the browser because of build steps/engineering (eg early jQuery source maps.)

Maybe another solution is to improve reporting via forms, or just let there be enough complaints to warrant the extra code?