[GitHub] accumulo pull request: Accumulo 3652 Refactor for slf4j string sub...

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request: Accumulo 3652 Refactor for slf4j string sub...

ctubbsii
GitHub user thormanrd opened a pull request:

    https://github.com/apache/accumulo/pull/32

    Accumulo 3652 Refactor for slf4j string substitution.  Rebased from master, style and merge corrections

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/thormanrd/accumulo-3652 ACCUMULO-3652

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/32.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #32
   
----
commit de2785064798304266afdb1d88ab5fe01ba47f57
Author: Bob Thorman <[hidden email]>
Date:   2015-04-09T17:23:35Z

    ACCUMULO-3652 refactor for slf4j all packages except gc, master, monitor and server-base

commit 679af5d2fe3b7b4b22568f1dffafaf1e9a255dd4
Author: Bob Thorman <[hidden email]>
Date:   2015-04-09T18:48:06Z

    ACCUMULO-3652 refactor for slf4j for all packages except server-base and test

commit 6f512a0ff7714b24cf80d76dae83823ae3c03cd0
Author: Bob Thorman <[hidden email]>
Date:   2015-04-09T20:21:22Z

    ACCUMULO-3652 refactor for slf4j for every package except accumulo-test

commit a027ef8deb454e91b7dadc6dfd755d3a682ad3c2
Author: Bob Thorman <[hidden email]>
Date:   2015-04-10T15:40:01Z

    ACCUMULO-3652 refactored for slf4j for all packages except Shell

commit 6d632273f70ca59f9e671d2256dcae5682ee94fd
Author: Bob Thorman <[hidden email]>
Date:   2015-04-14T15:22:40Z

    First round of revisions from the review board.

commit 812a81275036968a1e58546b7e69f180b6f1a0d8
Author: Bob Thorman <[hidden email]>
Date:   2015-04-16T15:59:34Z

    ACCUMULO-3652 fixed style errors and merge conflicts

commit 861494f8ec0a677d5fdf5fbe979399137f895dbc
Author: Bob Thorman <[hidden email]>
Date:   2015-04-21T15:54:16Z

    ACCUMULO-3652 Whitespace corrections, merge conflicts resolution

commit ef0483623ce85f95290186ddbe91aca7b9692922
Author: Bob Thorman <[hidden email]>
Date:   2015-04-23T15:18:46Z

    ACCUMULO-3652 style corrections after mvn clean verify from merge with master

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request: Accumulo 3652 Refactor for slf4j string sub...

ctubbsii
Github user joshelser commented on the pull request:

    https://github.com/apache/accumulo/pull/32#issuecomment-95655661
 
    Thanks for continuing to work on this, @thormanrd. There still seem to be a bunch of unwanted changes in here. core/src/main/java/org/apache/accumulo/core/cli/MapReduceClientOnDefaultTable.java and core/src/main/java/org/apache/accumulo/core/cli/MapReduceClientOnRequiredTable.java changed the name of the class a static method was called on. I also see a bunch of changes to the machine-generated thrift classes (e.g. core/src/main/java/org/apache/accumulo/core/client/impl/thrift/TDiskUsage.java). It seems like an IDE might have been a little overzealous?
   
    Can you try to trim out the changes which were made that aren't related to slf4j, please?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] accumulo pull request: Accumulo 3652 Refactor for slf4j string sub...

Bob Thorman
Sure, how do I trim them out?  They are the result of the 'git pull
--rebase¹ command.  When I tried to create the PR without some of them
earlier (PR¹s #30, #31) I got a compiler error from Jenkins.

v/r
Bob Thorman
Principal Big Data Engineer
AT&T Big Data CoE
2900 W. Plano Parkway
Plano, TX 75075
972-658-1714






On 4/23/15, 12:06 PM, "joshelser" <[hidden email]> wrote:

>Github user joshelser commented on the pull request:
>
>    https://github.com/apache/accumulo/pull/32#issuecomment-95655661
>  
>    Thanks for continuing to work on this, @thormanrd. There still seem
>to be a bunch of unwanted changes in here.
>core/src/main/java/org/apache/accumulo/core/cli/MapReduceClientOnDefaultTa
>ble.java and
>core/src/main/java/org/apache/accumulo/core/cli/MapReduceClientOnRequiredT
>able.java changed the name of the class a static method was called on. I
>also see a bunch of changes to the machine-generated thrift classes (e.g.
>core/src/main/java/org/apache/accumulo/core/client/impl/thrift/TDiskUsage.
>java). It seems like an IDE might have been a little overzealous?
>    
>    Can you try to trim out the changes which were made that aren't
>related to slf4j, please?
>
>
>---
>If your project is set up for it, you can reply to this email and have
>your
>reply appear on GitHub as well. If your project does not have this feature
>enabled and wishes so, or if the feature is enabled but not working,
>please
>contact infrastructure at [hidden email] or file a JIRA ticket
>with INFRA.
>---

Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] accumulo pull request: Accumulo 3652 Refactor for slf4j string sub...

Josh Elser
https://github.com/thormanrd/accumulo-3652/commit/861494f8ec0a677d5fdf5fbe979399137f895dbc 
includes some changes to the class used in a static method call.

https://github.com/thormanrd/accumulo-3652/commit/6d632273f70ca59f9e671d2256dcae5682ee94fd 
seems to contain the majority of the rest.

A rebase won't help here. You would need to fix these commits to undo
those changes.

THORMAN, ROBERT D wrote:

> Sure, how do I trim them out?  They are the result of the 'git pull
> --rebase¹ command.  When I tried to create the PR without some of them
> earlier (PR¹s #30, #31) I got a compiler error from Jenkins.
>
> v/r
> Bob Thorman
> Principal Big Data Engineer
> AT&T Big Data CoE
> 2900 W. Plano Parkway
> Plano, TX 75075
> 972-658-1714
>
>
>
>
>
>
> On 4/23/15, 12:06 PM, "joshelser"<[hidden email]>  wrote:
>
>> Github user joshelser commented on the pull request:
>>
>>     https://github.com/apache/accumulo/pull/32#issuecomment-95655661
>>
>>     Thanks for continuing to work on this, @thormanrd. There still seem
>> to be a bunch of unwanted changes in here.
>> core/src/main/java/org/apache/accumulo/core/cli/MapReduceClientOnDefaultTa
>> ble.java and
>> core/src/main/java/org/apache/accumulo/core/cli/MapReduceClientOnRequiredT
>> able.java changed the name of the class a static method was called on. I
>> also see a bunch of changes to the machine-generated thrift classes (e.g.
>> core/src/main/java/org/apache/accumulo/core/client/impl/thrift/TDiskUsage.
>> java). It seems like an IDE might have been a little overzealous?
>>
>>     Can you try to trim out the changes which were made that aren't
>> related to slf4j, please?
>>
>>
>> ---
>> If your project is set up for it, you can reply to this email and have
>> your
>> reply appear on GitHub as well. If your project does not have this feature
>> enabled and wishes so, or if the feature is enabled but not working,
>> please
>> contact infrastructure at [hidden email] or file a JIRA ticket
>> with INFRA.
>> ---
>
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #32: Accumulo 3652 Refactor for slf4j string substitution. Re...

ctubbsii
In reply to this post by ctubbsii
Github user milleruntime commented on the issue:

    https://github.com/apache/accumulo/pull/32
 
    I rebased @thormanrd 's branch against latest master here: https://github.com/milleruntime/accumulo/tree/accumulo-3652
   
    @thormanrd would you be able to merge my branch into yours if I make a pull request against your branch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #32: Accumulo 3652 Refactor for slf4j string substitution. Re...

ctubbsii
In reply to this post by ctubbsii
Github user ctubbsii commented on the issue:

    https://github.com/apache/accumulo/pull/32
 
    Once we switch to GitBox, we should just close this issue, and work from the rebase'd version, rather than try to merge and update this PR. You don't actually want to merge, anyway, then it will bring both old and new (rebase'd) history.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---