Not picking on this pull request in particular, but I find stylistic
feedback to be a poor use of code reviews. We should use humans to
identify logic errors, missing tests, and higher-level code quality
(HashCode vs. byte[], Objects.equals, etc.). We should use machines to
codify lower-level issues (whitespace, brace placement, etc.) or just
let it go. Could the reviewers making large numbers of stylistic
comments investigate enabling additional Checkstyle violations to
automate these suggestions?
On Wed, Oct 08, 2014 at 06:53:47PM -0700, Adrian Cole wrote:
> Hi, team.
>
> Having done sweeping change across a dozen providers I totally feel this
> pain.
>
> In short, we know there are terrible malpractice like abstract builder for
> one field in existing code, yet we don't do anything about it. Then, when
> someone new comes in, somewhere between us telling them to delete extra
> spaces, we ack that the thing we know sucks is something they pasted.
>
> It is reasonable for them to not know the right answer or to be overwhelmed
> by what seems to be a timesoaking response to an innocent change.
>
> It isn't hard to fixup the existing providers so that they are already good
> examples by the time someone goes to change them. I.e. they don't carry
> behavior we know is awful. I tried to do some of that when I did the
> unasyncing.
>
> This stuff happens, basically a norm of tech debt. Now that we have
> checkstyle sorted, perhaps we can start paying down this sort of debt?
>
> I will volunteer to undebt one provider for each provider someone else
> volunteers for. Anyone with me?
>
> -A
> ---------- Forwarded message ----------
> From: "inbar stolberg" <notificati...@github.com>
> Date: Oct 8, 2014 4:51 PM
> Subject: Re: [jclouds] cinder availability zones api + list call
> implemented (#560)
> To: "jclouds/jclouds" <jclo...@noreply.github.com>
> Cc:
>
> In
> apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/extensions/ExtensionNamespaces.java:
>
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +package org.jclouds.openstack.cinder.v1.extensions;
> > +
> > +/**
> > + * Extension namespaces
> > + */
> > +public final class ExtensionNamespaces {
> > +
> > + /**
> > + * Admin Action extension
> > + */
> > + public static final String ADMIN_ACTIONS =
> > "http://docs.openstack.org/ext/admin-actions/api/v1.1";;
> > +
>
> 90% of this code is copy paste of existing code from the same branch not
> old code either ...
> I have no problem fixing this but doesnt seem like there is cosistency in
> the code if i can copy paste some thing and get rejected over 100 lines....
> :/
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/jclouds/jclouds/pull/560/files#r18620246>.
--
Andrew Gaul
http://gaul.org/
