commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [commons-dbcp] branch master updated: [DBCP-547] Add a ConnectionFactory class name setting for BasicDataSource.createConnectionFactory() #33.
Date Wed, 10 Jul 2019 14:49:08 GMT
On Tue, Jul 9, 2019 at 4:59 AM Mark Thomas <markt@apache.org> wrote:

> On 08/07/2019 21:42, Gary Gregory wrote:
> > On Mon, Jul 8, 2019 at 11:23 AM Mark Thomas <markt@apache.org> wrote:
> >
> >> On 08/07/2019 16:14, ggregory@apache.org wrote:
> >>> This is an automated email from the ASF dual-hosted git repository.
> >>>
> >>> ggregory pushed a commit to branch master
> >>> in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
> >>>
> >>>
> >>> The following commit(s) were added to refs/heads/master by this push:
> >>>      new 8a579d3  [DBCP-547] Add a ConnectionFactory class name setting
> >> for BasicDataSource.createConnectionFactory() #33.
> >>> 8a579d3 is described below
> >>>
> >>> commit 8a579d304595853012ccf8c2bc93022c383a35bb
> >>> Author: Gary Gregory <gardgregory@gmail.com>
> >>> AuthorDate: Mon Jul 8 11:13:58 2019 -0400
> >>>
> >>>     [DBCP-547] Add a ConnectionFactory class name setting for
> >>>     BasicDataSource.createConnectionFactory() #33.
> >>>
> >>>     - Update version from 2.6.1-SNAPSHOT to 2.7.0 since this commits
> adds
> >>>     new public APIs in BasicDataSource.
> >>>     - Provide an alternative implementation from the PR
> >>>     https://github.com/apache/commons-dbcp/pull/33 WRT to String value
> >>>     handling. The handling of empty string for the new APIs is made
> >>>     consistent with other String APIs instead of what is done in PR 33.
> >>>     - Formatted new class TesterConnectionFactory differently from the
> PR
> >>>     and sorted its methods.
> >>
> >> This appears to have used an indent of 4 spaces for continuation lines
> >> rather than 8, making the code harder to read.
> >>
> >
> > I'm pretty sure most of the code base does not use super-wide 8 spaces
> for
> > indentation (for example between an if-statement and its body.
>
> I am not talking about standard indentation of code blocks, for which |I
> agree 4 spaces is thr standard. I am talking about indentation *for
> continuation lines*. Take the following example:
>
> >>> diff --git
> a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
> >> b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
> >>> index 7c46359..3a3d065 100644
> >>> --- a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
> >>> +++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
> >>> @@ -109,7 +109,7 @@ public class BasicDataSource implements DataSource,
> >> BasicDataSourceMXBean, MBean
> >>>      }
> >>>
> >>>      protected static void validateConnectionFactory(final
> >> PoolableConnectionFactory connectionFactory)
> >>> -            throws Exception {
> >>> +        throws Exception {
> >>>          PoolableConnection conn = null;
> >>>          PooledObject<PoolableConnection> p = null;
> >>>          try {
>
> I've edited the code to avoid line wrapping in email but the original was:
>
> protected static void validateConnectionFactory(final PCF factory)
>         throws Exception {
>     PoolableConnection conn = null;
>     PooledObject<PoolableConnection> p = null;
>     try {
>
> Note how the "throws Exception {" line is indented 8 spaces rather than
> 4 because it is a continuation of the line above.
>
> After the reformatting this became:
>
> protected static void validateConnectionFactory(final PCF factory)
>     throws Exception {
>     PoolableConnection conn = null;
>     PooledObject<PoolableConnection> p = null;
>     try {
>
> The continuation line is now only indented 4 spaces. This aligns it with
> the contained code block making it visually harder to separate the two.
>
> I quickly skimmed through the DBCP code base. I didn't find that many
> continuation lines but those that I did all used an 8 space indent.


Without arguing about the merits of one kind of formatting vs. another...
If you can configure the Eclipse formatter to do that, I'd consider it,
otherwise, I'm not into what I'd call "artisanal formatting" ;-)

Gary




> mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message