Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/231
Merged. 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
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
looks good
---
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
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/231
@sebbASF What do you think about the latest changes? Is this pull request
ready for merging?
Thanks, Pascal
---
If your project is set up for it, you can reply to this email
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10714214/badge)](https://coveralls.io/builds/10714214)
Coverage increased (+0.03%) to 94.597% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10714214/badge)](https://coveralls.io/builds/10714214)
Coverage increased (+0.03%) to 94.597% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10694580/badge)](https://coveralls.io/builds/10694580)
Coverage decreased (-0.009%) to 94.559% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10694580/badge)](https://coveralls.io/builds/10694580)
Coverage decreased (-0.009%) to 94.559% when pulling
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
Also it occurs to me that ProcessorArch and ProcessorType could perhaps be
subtypes of Processor (Arch and Type). They don't really have an independent
existence, so do they need separate
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
Agreed it's not necessary to have the isXXX methods.
However it makes the user code shorter and simpler.
Currently the code is:
```
Processor processor =
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10673667/badge)](https://coveralls.io/builds/10673667)
Coverage increased (+0.02%) to 94.589% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10673667/badge)](https://coveralls.io/builds/10673667)
Coverage increased (+0.02%) to 94.589% when pulling
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
I'm not sure, if we really need the isXXX methods in the Processor class,
because i can directly equal the enums.
---
If your project is set up for it, you can reply to this email and have
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
I thought the idea was to move the isXXX methods to the Processor class.
You could then say
```
Processor processor = ArchUtils.getProcessor();
processor.is32Bit();
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10550474/badge)](https://coveralls.io/builds/10550474)
Coverage decreased (-0.02%) to 94.513% when pulling
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
In which case, I think they belong on the Processor class.
The user code would look like:
```
Processor proc = ArchUtils.getProcessor([String]);
if (proc != null) {
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
> There's no point in having the isXXX() methods any more.
There just need to be methods to get the Processor.
If necessary, the Processor class could have isxxx methods on it.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
There's no point in having the isXXX() methods any more.
There just need to be methods to get the Processor.
If necessary, the Processor class could have isxxx methods on it.
---
If
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
> If there is no database entry, return a Process instance with enums which
indicate "Unknown".
(Attribute enums would need an extra 'Unknown' entry).
I think it is better to
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
The new design is more flexible. So one could add Endianness for example.
[The code still does not check for duplicate values for the same key]
However I'm not sure that the chosen
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
I have created a new class ArchUtilsImproved.
@sebbASF This approach is more extendable. Is this implementation better
for you?
---
If your project is set up for it, you can reply to
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
I wonder why the architecture String constants are not an enum?
Also, the various addxxx() methods don't check if there is already an
entry, so it's possible to overwrite an existing
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/231
If the code is not going to allow updates, then there is no need to use
ConcurrentHashMap.
If updates are allowed at run-time, there is the possibility that different
results will be
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10496329/badge)](https://coveralls.io/builds/10496329)
Coverage decreased (-0.02%) to 94.519% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10496329/badge)](https://coveralls.io/builds/10496329)
Coverage decreased (-0.05%) to 94.483% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10496329/badge)](https://coveralls.io/builds/10496329)
Coverage decreased (-0.05%) to 94.483% when pulling
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
I changed the HashMap to a ConcurrentHashMap.
---
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
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10485308/badge)](https://coveralls.io/builds/10485308)
Coverage decreased (-0.05%) to 94.483% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10484777/badge)](https://coveralls.io/builds/10484777)
Coverage decreased (-0.05%) to 94.483% when pulling
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
I'm sorry. I removed the tabs. Style should be ok now.
---
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
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
Excellent @Tomschi
I'm dropping a note to the mailing list to ask for feedback from crypto
devs, as there could be some synergy.
I will play with the code at home, but one
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
Yes, the base of my code references to the commons crypto lib.
---
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
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
Looks better now @Tomschi did you use Crypto's code as reference? cc
@michael-o
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user michael-o commented on the issue:
https://github.com/apache/commons-lang/pull/231
@kinow The cheapest way is to produce two bundles, one for 32 bit and one
for 64 bit, if possible. The lopica source is useless, it is 15 years old.
Commons Crypto is better. hawtjni on
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
@michael-o
>What is the real pupose for this actually? The client should not care
about the arch at all.
I think @Tomschi use case is valid, where a client could need to know
Github user michael-o commented on the issue:
https://github.com/apache/commons-lang/pull/231
What is the real pupose for this actually? The client should not care about
the arch at all. The regex match is brittle. This will likely fail on ARM and
it fails here on Itanium with HP-UX
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/231
jira issue: https://issues.apache.org/jira/browse/LANG-1313
I plan to merge this tomorrow (if there are no objections).
---
If your project is set up for it, you can reply to
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10210615/badge)](https://coveralls.io/builds/10210615)
Coverage increased (+0.001%) to 94.53% when pulling
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
It's ok for me, i will change it.
---
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
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
Oh, good point @PascalSchumacher have no objection to it. We could probably
avoid a few misunderstandings that way. Happy with that too @Tomschi ?
---
If your project is set up for it, you can
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/231
I think this is a worthy addition.
In my experience people often do not read documentation. Maybe we should
use `IS_32_BIT_JVM` so there can be no confusion? Or is this is too
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
Gotcha, found this example
https://github.com/Tomschi/jacob-parent/blob/ec3f1c10169c26f14ee1f61bd6622c67a73e26fc/jacob/src/main/java/com/jacob/com/LibraryLoader.java#L202
Looks like a
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
I'am using it for the jacob-project. There i have to know, if it is a 32 or
64 bit architecture to load the correct dll library.
---
If your project is set up for it, you can reply to this
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
Thanks for updating the pull request @Tomschi.
I don't have a use case for this. I can see where it could be used, but I
don't have any project where I would use it. Code is clear,
Github user Tomschi commented on the issue:
https://github.com/apache/commons-lang/pull/231
Rewrite javadoc's.
---
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
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/10120247/badge)](https://coveralls.io/builds/10120247)
Coverage decreased (-0.006%) to 94.53% when pulling
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/231
The discussion mentioned by @kinow is here:
https://issues.apache.org/jira/browse/LANG-1145
---
If your project is set up for it, you can reply to this email and have your
reply
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/231
I remember a discussion about it some time ago.
The issue with this approach was that os.arch tells only the JVM arch, not
really OS arch.
If there is a strong use case for
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/231
[![Coverage
Status](https://coveralls.io/builds/9969109/badge)](https://coveralls.io/builds/9969109)
Coverage increased (+0.0007%) to 94.456% when pulling
48 matches
Mail list logo