phpbb / epv

Extension Pre-Validator
GNU General Public License v2.0
9 stars 17 forks source link

Suggestion: support for .editorconfig #82

Open LukeWCS opened 5 years ago

LukeWCS commented 5 years ago

Hi

EPV is a helpful tool, I like it.

There is one thing I would to suggest: to support (or ignore) the file .editorconfig. This is another standard and directly supported by GitHub, so it should be accepted by EPV too.

Informations about this standard:

https://editorconfig.org/

There you can see also, that .editorconfig is supported directly by several IDE's and editor's. For some who have no native support for it, there exists plugins.

DavidIQ commented 5 years ago

Extension packages don't need or use this file so it should not be included in extension packages. If EPV is rejecting it then that's how it should be.

LukeWCS commented 5 years ago

This is exactly what I do, I exclude this file via .gitattributes so the users don't get this file in a GitHub download. The only problem is, that EPV doesn't ignore it.

DavidIQ commented 5 years ago

I think that you're referring to when EPV is run directly against the repo, in which case you are correct and it should be ignored by EPV.

paul999 commented 5 years ago

It should not be ignored but EPV should give a error that development related files should not exists within the package (Keep in mind that EPV is ment at first for released packages). The same applies to other development related files like .gitignore, .travis, and travis/.

LukeWCS commented 5 years ago

@DavidIQ That's exactly what I meant, yes. It's the same like .gitignore and .gitattributes. These both files are also not needed for a phpBB Ext package and they are ignored by EPV.

LukeWCS commented 5 years ago

@paul999

(Keep in mind that EPV is ment at first for released packages)

Until now I thought a EPV run without errors is the first step needed that the Ext is accepted by the validation team. Then I misunderstood this.

After the messages from DavidIQ and from you Paul, I have read now the EPV page again and I think now I understand: there are two ways for running EPV. Method 1) directly against the GitHub repository. Method 2) before adding to the Extension Database. Is that correct? I never released (until now) an extension so I only know method 1.

Ok, but this violates not to my suggestion. For me EPV is very helpful to test things before I give an extension to the validation team.

Then I change my suggestion to this:

Method 1 (check GH Repo): the EPV should ignore .editorconfig, like .gitignore and .gitattrbutes. Method 2 (adding to Extension Database): the EPV should report any file which are not related to a extension package.

paul999 commented 5 years ago

No, you actually didn't misunderstand that, the goal of EPV is to provide a tool to give feedback on the most commonly made mistakes when making extensions. This is, in general, to be done when submitting it to the CDB at phpBB.com, and as such it is a release package. For the specific release package, there are rules on which the package should comply. EPV tries to validate a few of those rules, but currently not all of them.

As secondary function EPV also supports running it from a repo to provide early feedback. However, as the rules are made for release packages, there are specific cases for this function that EPV returns an error, but that error is only a issue when you are submitting it as release. In general we tell users to ignore those specific errors (One of them currently in there is a packaging specific requirement, see https://github.com/phpbb/epv/issues/33)

We decided some time ago that we write the validation based on the release package, and accept in specific cases that errors are sometimes false positive whe running from a repo.

Another thing you need to keep in mind that even if EPV gives an errors, this doesn't directly mean a extension will be denied. EPV is a tool to help authors, but also the extensions team at phpBB.com to provide easy feedback for standard mistakes and the extensions team validates on a case by case basis for that specific error if it an issue. (Ofcourse this doesn't apply to all errors returned by EPV. For example, fatal errors returned will also result in a deny. ERRORs will result in a deny in most cases, and notices are more to warn the validator about something that is happening and he should look at during validation)

LukeWCS commented 5 years ago

Thank you Paul for your detailed answer. :)

Another thing you need to keep in mind that even if EPV gives an errors, this doesn't directly mean a extension will be denied.

That's exactly what I misunderstood. So if I plan to make a validation request, the (only) warning related to .editorconfig is no problem then.