[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-09-05 Thread ameyjadiye
Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/55
  
@chtompki , I'm sorry for such short note, 2.x was for making .builder() 
static, PR seems ok for accepting in 1.2 release, you might want to go though 
jira as well.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-09-05 Thread chtompki
Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/55
  
Pardon...I don't see why this would necessitate a 2.X. Let me re-read.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-09-05 Thread chtompki
Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/55
  
Are we ready to do a change that would necessitate a 2.X?


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-09-05 Thread ameyjadiye
Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/55
  
@chtompki , If possible can you review and accept this ? I need this for 
TEXT-101.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-07 Thread ameyjadiye
Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/55
  
Yeah, will see if we can simplify API in 2.x .
I need opinion on the method I have given in this PR, According to me it's 
good addition to existing API but go through JIRA discussion once. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-07 Thread jbduncan
Github user jbduncan commented on the issue:

https://github.com/apache/commons-text/pull/55
  
Revisiting this in 2.x sounds good to me!

The only other thought I have is that I think this should go up as a new 
issue on Apache JIRA if it hasn't already.

Other than that, everything LGTM.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-07 Thread chtompki
Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/55
  
@jbduncan has a point here, but the code here does conform to the style of 
the existing code. So, I'd lean more towards the changes @ameyjadiye is 
proposing mainly because to re-work the code into a static `builder()` method 
pattern would require changes that would necessitate a major version change. We 
could, though, revisit this in the 2.X version if you with @jbduncan. Thoughts?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-06 Thread jbduncan
Github user jbduncan commented on the issue:

https://github.com/apache/commons-text/pull/55
  
@ameyjadiye I don't know if there's a convention in Apache Commons for 
preferring `new X.Builder()` over static factory methods like `X.builder()`, 
but if there isn't, then I would encourage changing the API so that instead of 
`new RandomStringGenerator.Builder()`, one can write the shorter and arguably 
more readable `RandomStringGenerator.builder()`.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-06 Thread chtompki
Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/55
  
Sure...I'll try to look at this in the coming day or so.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-06 Thread ameyjadiye
Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/55
  
@chtompki , @PascalSchumacher , can you please express your opinion on this 
, please check JIRA for detail discussion.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...

2017-07-01 Thread coveralls
Github user coveralls commented on the issue:

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

[![Coverage 
Status](https://coveralls.io/builds/12217890/badge)](https://coveralls.io/builds/12217890)

Coverage increased (+0.007%) to 97.312% when pulling 
**beb0b4615a94420cc86595c1f060711dd999af91 on ameyjadiye:TEXT-97** into 
**5e479dcd74dab262e5080991796395c3e29222b9 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org