search

unraid / webgui

Unraid Web UI
157 stars 70 forks source link

Add TravisCI/AppVeyor merge checks #321

Open realies opened 6 years ago

realies commented 6 years ago

One way of improving the quality of code is to use tools for continuous integration. They would make sure that code is written consistently, follows best practices and that it works. Please consider adding TravisCI, AppVeyor or why not both as pull request checking tools to improve the codebase quality.

limetech commented 6 years ago

Hey that would be great, but there is a saying: "money talks, bullshit walks". How about if you investigate which of those would be appropriate and actually code something up showing proof-of-concept?

realies commented 6 years ago

What do you mean? You need someone to configure the systems for you? Or you're having trouble understanding what they can do?

limetech commented 6 years ago

Sure, how about you configure it and explain how what it does and how it helps, and if looks like a good thing, we'll do it.

realies commented 6 years ago

My apologies, assumed you are aware of continuous integration as a practice or at least had the will to look it up and respond on topic. As aforementioned, tools for continuous integration help with making sure that code is written consistently, follows best practices and that it works.

Paraphrased with an example: If I make a pull request against the webgui repository and Travis CI or AppVeyor is integrated with it, GitHub would let them know of the pull request, they would review the code change and make sure it follows defined guidelines in terms of good practices and code formatting, the repository would get cloned in an isolated environment, the changes would get applied and potential unit tests would be performed to determine if existing functionality is intact and new additions work as expected. The pull request would reflect the result from of the checks: successful or failed. If the latter, a link would guide to more details about the issue(s) within the respective CI tool.

The reason for proposing CI is that the codebase has integrations by multiple people that follow different coding styles. CI would be able to help maintain a single convention and indicate whenever it has not been followed. As a first step, such a tool could be used to indicate small things such as the usage of ' vs " for different parts of the code, strict use of { } for one-line if statements or the use of const for variables that have never changed (and potentially would never have to) down to logical tests such as the array that would never get populated if $post['confName'] is not defined.

limetech commented 6 years ago

or at least had the will to look it up and respond on topic.

Do you see how that phrase makes you sound like a dick?

Anyway, sure I know about that, but it assumes I give a crap about code style. I don't really care if someone uses tabs and someone else uses spaces. It just doesn't interest me. Sure it's nice to be consistent, but I'd much rather have code from people who are happy to contribute, knowing their work is not going to be nit picked over trivialities. This is the nature of open source. If your last name isn't "Torvalds" then you have to overlook pedantic issues like this.

But hey, if you want to show us how to use these tools, I'm all for it.

realies commented 6 years ago

Tom, I have paid you money and am making attempts at improving your situation via means that are the nature of open source, which is free. Not making an effort to look up my suggestions and asking me to investigate which of those would be appropriate and actually code something up showing proof-of-concept does make you look like the dick in my eyes. Especially after sure, you know about this, implying that my time writing those three paragraphs has been wasted.

CI is not about making you give a crap about code style, it is not about you starting to care about someone using tabs or spaces and it is not about making you interested in it. It is for making sure everything is identical and that code requires less human effort to ensure it passes expected results and does not break any other functionality as part of the project. If you have not noticed, there are commits in the repository that deal with just this, ensuring code style is kept consistent. This is waste of human time no matter what you think or how much you give a crap. It is somehow contradictory that you would be happy for your business to continue (by having people that are happy to contribute) and not giving a damn insight into what is being proposed instead asking for a proof-of-concept in a tray for you to consider it appropriate or not.

Sure, assigning an array when it is not going to ever be used is a triviality. Stacking those trivialities for years is another thing that I hope you can acknowledge. You have no position to ask contributors that pay for your software to overlook pedantic issues when they are trying to help you. I hope you realise that your product can sell a lot more if it works better.

limetech commented 6 years ago

Look man, I'm not trying to get into a pissing contest about this. You still don't get it: the objection is not about your suggestions (they are good) or code (you obviously know what you're doing): it's how you present it.

limetech commented 6 years ago

Let me add one more thing: adding improvements to code and workflow are very important. But I have probably a dozen important thing things to work on. Everyone who has something important for me to work on thinks their thing is the most important thing to work on. It's real easy for someone to say, "hey, you should do this, it's important". My reply is, "If you think it's so important, and you are able, then you do it and show me what you did."