[GitHub] commons-text pull request #52: Test: Improved testcase coverage for StrBuild...

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text pull request #52: Test: Improved testcase coverage for StrBuild...

asfgit
GitHub user ameyjadiye opened a pull request:

    https://github.com/apache/commons-text/pull/52

    Test: Improved testcase coverage for StrBuilder

    added some missing flows to testcases plus some nice formatting for easy understanding.

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

    $ git pull https://github.com/ameyjadiye/commons-text test-StrBuilder

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

    https://github.com/apache/commons-text/pull/52.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 #52
   
----
commit 06e8b3dd06ee2f86490b2e94359d29d2fe04786c
Author: Amey Jadiye <[hidden email]>
Date:   2017-06-15T20:15:10Z

    test: Improved testcase coverage for StrBuilder

----


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

asfgit
Github user PascalSchumacher commented on the issue:

    https://github.com/apache/commons-text/pull/52
 
    Thanks!
   
    Please separate the formatting fixes and the test additions into different commits.
   
    Please stick to the existing code style for the new tests (no new line at the beginning and at the end of a test method; single new line between test methods).



---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Agreed with Pascal here, just for semantics on commit 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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Fixed all the code as suggested, Thanks.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
   
    [![Coverage Status](https://:/builds/12055561/badge)](https://:/builds/12055561)
   
    Changes Unknown when pulling **31a5cd173d118148b2a2d2dfac8c99f2d90a8a06 on ameyjadiye:test-StrBuilder** into ** on apache: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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Thanks for splitting the changes. :+1:
   
    But please also address the second part:
   
    >Please stick to the existing code style for the new tests (no new line at the beginning and at the end of a test method; single new line between test methods).
   
    Furthermore it would be very nice if you would clean up the history of the pull request by using `git rebase` once it is ready to merge.
   
    After you commit the style fixes for the new tests you should be able to clean-up the history like this:
   
    Use `git rebase -i head~5`. Choose to drop the first two commits (06e8b3dd06ee2f86490b2e94359d29d2fe04786c and 27e110b37f8f1e1cceee18ab614424f8a807a747). Squash d23bd1db8c0f21f62006b8cf0f41eb0588997a1e and the last commit. Afterwards use `push -f` to update the pull request.
   
    After this there will be only two commits (one with the style changes and one with the new tests) and the commit history of commons-text will be pristine. :smile:


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
   
    [![Coverage Status](https://:/builds/12061474/badge)](https://:/builds/12061474)
   
    Changes Unknown when pulling **242ad5e6cda2cc62be4e1b8fccedd7922ecdc44e on ameyjadiye:test-StrBuilder** into ** on apache: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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    @PascalSchumacher This seems nice trick , but somehow didn't worked for me and ended up just pushing latest commit, can you try this once ? I have given you write access for exact this repository.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
   
    [![Coverage Status](https://:/builds/12062509/badge)](https://:/builds/12062509)
   
    Coverage increased (+0.4%) to 97.317% when pulling **eb25d61ca6bd845815bb30feb2ba0cf9f6e106ff on ameyjadiye:test-StrBuilder** into **9f6e5e45e925511d0fdbf59bb264aafa6b91f997 on apache: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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Thanks. I was not as easy as I expected (I had to merge).
   
    I followed these steps:
   
    `git rebase -i head~5`
   
    Edit the rebase-todo-to to contain:
    ```
    drop 06e8b3d test: Improved testcase coverage for StrBuilder
    drop  27e110b revert to master
    pick d23bd1d Test: Improved testcase coverage for StrBuilder
    squash 242ad5e Style fixes for StrBuilderTest
    pick 31a5cd1 Fixed formatting for StrBuilder
    ```
   
    Edit merge conflict.
   
    `git rebase --continue`
   
    Editmerge conflicts (nothing to do, just confirm merge was good).
   
    `git rebase --continue`
   
    `push -f`
   
    Please remove my access to the repository. Thanks!


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Cool, seems clean now :+1: thanks Pascal. Trick will be certainly beneficial in future to me 😉


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Looking good! :+1:


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text issue #52: Test: Improved testcase coverage for StrBuilder

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

    https://github.com/apache/commons-text/pull/52
 
    Thanks Amey.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] commons-text pull request #52: Test: Improved testcase coverage for StrBuild...

asfgit
In reply to this post by asfgit
Github user asfgit closed the pull request at:

    https://github.com/apache/commons-text/pull/52


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...