[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10714214) Coverage increased (+0.03%) to 94.597% when pulling **29941f409c129fb9eccd4359cddd03e7305de708 on Tomschi:master** into **5d6c176388e417869421adf2eb21ac5c291f0884 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10714214) Coverage increased (+0.03%) to 94.597% when pulling **29941f409c129fb9eccd4359cddd03e7305de708 on Tomschi:master** into **5d6c176388e417869421adf2eb21ac5c291f0884 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10694580) Coverage decreased (-0.009%) to 94.559% when pulling **a9c6f59b005c8e36980165b98f5dedbc4329438e on Tomschi:master** into **5d6c176388e417869421adf2eb21ac5c291f0884 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10694580) Coverage decreased (-0.009%) to 94.559% when pulling **a9c6f59b005c8e36980165b98f5dedbc4329438e on Tomschi:master** into **5d6c176388e417869421adf2eb21ac5c291f0884 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 classes? --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 = ArchUtils.getProcessor();
if (ProcessorArch.BIT_32.equals(processor.getProcessorArch())) {}
if (ProcessorType.PPC.equals(processor.getProcessorType())) {}
```
It would become:
```
Processor processor = ArchUtils.getProcessor();
if (processor.is32Bit()) {}
if (processor.isPPC()) {}
```
As well as being longer, in the first case one has to remember to whether
to use ProcessorArch or ProcessorType
---
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.
---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10673667) Coverage increased (+0.02%) to 94.589% when pulling **dbdb045c46294191245672bad6f062ba00fbfb70 on Tomschi:master** into **5d6c176388e417869421adf2eb21ac5c291f0884 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10673667) Coverage increased (+0.02%) to 94.589% when pulling **dbdb045c46294191245672bad6f062ba00fbfb70 on Tomschi:master** into **5d6c176388e417869421adf2eb21ac5c291f0884 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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(); processor.isPPC(); etc. ``` --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10550474) Coverage decreased (-0.02%) to 94.513% when pulling **85ea6ce337b63392d37747a961e199ff6df5ad48 on Tomschi:master** into **d43e1d01981d321b85a8ccae788da4d818548dbe 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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) {
if (proc.isX86() && proc.isPPC()) {
}
} // else not supported
```
At present the same code is
```
if (ArchUtils.isSupported([String])) {
if (ArchUtils.isX86([String]) && ArchUtils.isPPC([String]) )
}
}
```
I think that looks more complicated.
Also all the isXXX methods have to be overloaded with an optional string
parameter.
And the map is accessed at least 3 times.
---
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.
---
[GitHub] commons-lang issue #231: Evaluate Architecure
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. This methods are for me an easy and null safe way for evaluating. Further, this methods are the reason, why i have done this pull request. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 return null, if the Processor doesn't exists. I added UNKNOWN to both enums, but UNKNOWN can also be used for a existing Processor. So it is hard to evaluate, if the Processor exists or not, when the properties UNKNOWN are used. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 names fully agree with the common usage. For example, architecture is often used for CISC, RISC etc. The enums need to be carefully documented so their domain is clear. I'm not sure it makes sense to have combined methods such as isPPC64JVM() which asserts that it is a PPC and 64bit JVM. Also there is a usage issue: a false response may mean that the string is unknown. Even though there only two ProcessorArch values currently, one cannot assume that a string which is not 64 bit must therefore be 32 bit. I wonder if it would not be simpler to just return the Processor entry. The caller can then check the attributes in any combinations they want. If there is no database entry, return a Process instance with enums which indicate "Unknown". (Attribute enums would need an extra 'Unknown' entry). --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 value. This should be easy to fix (I suggest a common method to update the map called by all the others). === The current design assumes that a given OS string can only be in one of the classifications. Will that always be the case? At present checking for 64bit JVMs means knowing which classes are 64 bit etc. It would be possible (but awkward) to add a method for isIntel. Perhaps consider using separate flags for each attribute, instead of names for specific combinations of attributes. This would make it much easier to add new attributes, e.g. CISC,RISC --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 obtained at different times. Alternatively, it's potentially possible to allow updates before first use of the data, but disallow them later. This is the approach taken in Commons Validator http://commons.apache.org/proper/commons-validator/xref/org/apache/commons/validator/routines/DomainValidator.html This uses a flag to allow initial updates. Users have to get an instance before using the class; this resets the flag to stop further changes. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10496329) Coverage decreased (-0.02%) to 94.519% when pulling **918195ca0d1b472bf66d035bd54a86df8096b55f on Tomschi:master** into **d43e1d01981d321b85a8ccae788da4d818548dbe 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10496329) Coverage decreased (-0.05%) to 94.483% when pulling **918195ca0d1b472bf66d035bd54a86df8096b55f on Tomschi:master** into **d43e1d01981d321b85a8ccae788da4d818548dbe 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10496329) Coverage decreased (-0.05%) to 94.483% when pulling **918195ca0d1b472bf66d035bd54a86df8096b55f on Tomschi:master** into **d43e1d01981d321b85a8ccae788da4d818548dbe 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10485308) Coverage decreased (-0.05%) to 94.483% when pulling **6eb918e78fd2b12da9cebeeedf13b882096a3502 on Tomschi:master** into **d43e1d01981d321b85a8ccae788da4d818548dbe 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10484777) Coverage decreased (-0.05%) to 94.483% when pulling **6eb918e78fd2b12da9cebeeedf13b882096a3502 on Tomschi:master** into **d43e1d01981d321b85a8ccae788da4d818548dbe 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 thing that I noticed is that it seems to contain tabs... minor nit pick, but could you check checkstyle and make sure the code is not introducing any warnings / errors there, please? That way it will be easier to merge the PR. Thanks! B --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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. 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 has decent detection code. It is work checking. But the current approach is way too simple. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 the arch before loading a certain library, and we had another issue submitted LANG-1145 with similar requirement. >The regex match is brittle. This will likely fail on ARM and it fails here on Itanium with HP-UX for os.arch IA64N which is a 32 bit VM. Note taken, perhaps before merging we can try to cover more archs, like this list: * http://lopica.sourceforge.net/os.html There is another place where arch is used within Commons too: * https://github.com/apache/commons-crypto/blob/158be0644c353a617427ab190a4f09082cda42ac/src/main/java/org/apache/commons/crypto/OsInfo.java#L28 We can possibly look at how crypto is using it, and if we could maybe use a similar approach here. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 for `os.arch` `IA64N` which is a 32 bit VM. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10210615) Coverage increased (+0.001%) to 94.53% when pulling **3e335fd2046ad3cf6bd84456b836d8025998c321 on Tomschi:master** into **30c85ad05363767deeefee577063c2c432b971d4 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 paranoid? What do you think? --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 valid use case. I'm all right with merging it, my +0. Let's now wait to see what others think about it. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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, javadocs have been updated :-) So will leave it here for others to review, comment, and maybe perhaps. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/10120247) Coverage decreased (-0.006%) to 94.53% when pulling **a5945e7b0b722aab1693d3912b76c0042fc74cee on Tomschi:master** into **b715d18f09591c510dded3a447a3ad1aacfa2173 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
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 these properties, we would have to be careful with the javadocs. Right now the javadocs don't explicitly tell the user that it is the JVM arch, and not the OS arch. Other approaches involve reflection or JNI, to find out the OS arch, but I think that is a bit too tricky to get it done correctly. --- 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. ---
[GitHub] commons-lang issue #231: Evaluate Architecure
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/231 [](https://coveralls.io/builds/9969109) Coverage increased (+0.0007%) to 94.456% when pulling **13df6b084b0ac64e291d10f7ecdb4dc85e7cde91 on Tomschi:master** into **ab25f67348d4885620df86497889c1826f013a73 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. ---
