3
\$\begingroup\$

I made a review of the comments on some code, and the user said something about that is the way that StyleCop mandates and I said they should mention that in the post, it wouldn't change my review, from what I see, StyleCop does more damage than it does fix things. but should I do something different with my review?

Is my review still valid even though it contradicts a tool that the user is currently using? does my review become off topic?

Question in Question : PDF script tool to convert documents to PDF and add fields and scripts


My Answer

\$\endgroup\$
  • \$\begingroup\$ Your review is your opinion, I don't see how it can be invalid. Whether or not people agree with you though, well that's their opinion. \$\endgroup\$ – Simon Forsberg Jun 6 '16 at 12:37
  • \$\begingroup\$ @SimonForsberg does that clarify a little bit more what I am thinking/asking? \$\endgroup\$ – Malachi Jun 6 '16 at 12:39
  • \$\begingroup\$ A review is rarely "off-topic", but considering the requirement of using StyleCop, your review becomes less relevant. \$\endgroup\$ – Simon Forsberg Jun 6 '16 at 12:41
8
\$\begingroup\$

The default StyleCop rules do run counter to VS2015's recommended style guide, and you can point that out if you wish. That said, if OP is using StyleCop, it's part of a requirement (and should be treated as much of a requirement as using a thread to solve Sieve is in a particular question).

I wouldn't say StyleCop is bad - it's great at enforcing consistency - and I don't think StyleCop being good or bad should found the basis of a review, but rather be a single point in that review.

I think it would be worth mentioning to OP that his rules run counter to the .NET guidelines, but if that is what his team is using, then that's what he's going to have to stick with.


Reading your specific answer, I would say that while I agree with you, I'm not sure what you've put there is substantive enough for a review given that OP is using StyleCop. Your answer ultimately comes down to an opinion, though, rather than a review that actually adds value to the OP's code, IMO.

I do think OP should have mentioned he was using StyleCop in his OP.

\$\endgroup\$
4
\$\begingroup\$

IMO, no.

If the user has a constraint that, for this particular code, they need to use Stylecop then that's a legitimate reason not to use your advice, but that doesn't make your review any less correct.

\$\endgroup\$
3
\$\begingroup\$

As the OP of the question in question, my reason for downvoting the answer in question was that it was not a constructive review of the code. Removing the documentation does nothing to improve the code itself, and while certain things (the enums for example) might read more naturally without the comments interjected, the enums or other pieces of code are not coded any better or worse solely because documentation is or is not present.

With that being said, after the fact, it was reconsidered, and summary comments have been removed considering that this is an end-user application and not an API. My concern with adding the comments in the first place is that I am a summer intern, and the IT team (not developers) will be responsible for maintaining the project after I leave. I wanted it to be as self-explanatory as possible, but my naming conventions were already doing that, and many of the summary comments were redundant.

I would remove the downvote from the answerer's answer, but the system is not permitting me to do so (it has been two days). As that is the case, I will accept what I assume was a retaliatory downvote on my question.

\$\endgroup\$
  • 3
    \$\begingroup\$ The answer got edited. Feel free to revert. Thanks to @Malachi for forgetting to properly capitalize his review in the first place ^^ \$\endgroup\$ – Mast Jun 6 '16 at 13:01
  • 2
    \$\begingroup\$ thank you for taking the time to explain. it is very appreciated. I have also updated my answer and hopefully you find it a little more helpful than it was before. \$\endgroup\$ – Malachi Jun 6 '16 at 13:07

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .