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

Suggestion: support for .editorconfig #82

Open
LukeWCS opened this issue Sep 26, 2018 · 8 comments
Open

Suggestion: support for .editorconfig #82

LukeWCS opened this issue Sep 26, 2018 · 8 comments

Comments

@LukeWCS
Copy link

LukeWCS commented Sep 26, 2018

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.

@LukeWCS LukeWCS changed the title Suggestion: support for .editorconfig Suggestion: support for .editorconfig Sep 26, 2018
@DavidIQ
Copy link
Member

DavidIQ commented Sep 26, 2018

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.

@DavidIQ DavidIQ closed this as completed Sep 26, 2018
@LukeWCS
Copy link
Author

LukeWCS commented Sep 26, 2018

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

DavidIQ commented Sep 26, 2018

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.

@DavidIQ DavidIQ reopened this Sep 26, 2018
@paul999
Copy link
Member

paul999 commented Sep 26, 2018

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

LukeWCS commented Sep 26, 2018

@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
Copy link
Author

LukeWCS commented Sep 26, 2018

@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
Copy link
Member

paul999 commented Sep 26, 2018

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 #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
Copy link
Author

LukeWCS commented Sep 26, 2018

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.

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

3 participants