You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/11/02 22:02:19 UTC

[GitHub] [commons-bcel] nbauma109 opened a new pull request, #163: added accessors to model and unit tests, javadoc comments

nbauma109 opened a new pull request, #163:
URL: https://github.com/apache/commons-bcel/pull/163

   - Added getters to module classes which had no accessors for their private fields (removed TODO comments)
   - compactClassName only seemed to make sense for uses/provides, opens/requires/exports already had dot package separator in the constant pool
   - Other getters added for various attributes


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] codecov-commenter commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1337513547

   # [Codecov](https://codecov.io/gh/apache/commons-bcel/pull/163?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#163](https://codecov.io/gh/apache/commons-bcel/pull/163?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (07dc2b7) into [master](https://codecov.io/gh/apache/commons-bcel/commit/587c9869e2dd9abc5dae70457c44f1ddabd0ef00?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (587c986) will **increase** coverage by `15.89%`.
   > The diff coverage is `70.00%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master     #163       +/-   ##
   =============================================
   + Coverage     44.92%   60.82%   +15.89%     
   - Complexity     2516     3643     +1127     
   =============================================
     Files           362      363        +1     
     Lines         15567    15698      +131     
     Branches       1920     1951       +31     
   =============================================
   + Hits           6994     9548     +2554     
   + Misses         7891     5272     -2619     
   - Partials        682      878      +196     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/commons-bcel/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...rc/main/java/org/apache/bcel/classfile/Method.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL01ldGhvZC5qYXZh) | `69.23% <50.00%> (-1.27%)` | :arrow_down: |
   | [...rc/main/java/org/apache/bcel/classfile/Module.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL01vZHVsZS5qYXZh) | `61.38% <60.00%> (+13.71%)` | :arrow_up: |
   | [...java/org/apache/bcel/classfile/ModuleRequires.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL01vZHVsZVJlcXVpcmVzLmphdmE=) | `62.96% <62.50%> (-0.68%)` | :arrow_down: |
   | [...java/org/apache/bcel/classfile/ModuleProvides.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL01vZHVsZVByb3ZpZGVzLmphdmE=) | `65.78% <68.75%> (+65.78%)` | :arrow_up: |
   | [.../java/org/apache/bcel/classfile/ModuleExports.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL01vZHVsZUV4cG9ydHMuamF2YQ==) | `66.66% <71.42%> (+14.94%)` | :arrow_up: |
   | [...in/java/org/apache/bcel/classfile/ModuleOpens.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL01vZHVsZU9wZW5zLmphdmE=) | `66.66% <71.42%> (+66.66%)` | :arrow_up: |
   | [src/main/java/org/apache/bcel/classfile/Code.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0NvZGUuamF2YQ==) | `70.70% <100.00%> (+4.04%)` | :arrow_up: |
   | [.../java/org/apache/bcel/classfile/FieldOrMethod.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0ZpZWxkT3JNZXRob2QuamF2YQ==) | `55.38% <100.00%> (+2.92%)` | :arrow_up: |
   | [...main/java/org/apache/bcel/classfile/JavaClass.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0phdmFDbGFzcy5qYXZh) | `71.23% <100.00%> (+8.82%)` | :arrow_up: |
   | [src/main/java/org/apache/bcel/util/ClassPath.java](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvdXRpbC9DbGFzc1BhdGguamF2YQ==) | `42.40% <0.00%> (-0.52%)` | :arrow_down: |
   | ... and [262 more](https://codecov.io/gh/apache/commons-bcel/pull/163/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] nbauma109 commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
nbauma109 commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1303392399

   Ah ok thanks I was missing the context of all this 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] garydgregory commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1302509608

   > I will update the javadocs with description and since tag. Didn't you see the unit tests ?
   
   Tests: Sorry, I must have gotten this PR mixed up with the other one currently open for more tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] nbauma109 closed pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
nbauma109 closed pull request #163: added accessors to model and unit tests, javadoc comments
URL: https://github.com/apache/commons-bcel/pull/163


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] nbauma109 commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
nbauma109 commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1303047659

   These getter are meant to avoid boilerplate code using getAttributes so they're not completely indispensable, maybe except the getter on indexes of the module which could be accessed by reflection but that would be horrible. There are a lot of use cases of showing class attributes in graphical components or ASCII table in console. If we parse a class file, the elements must be accessible inside it, not just a toString representation of module components, that one would have to reparse.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] nbauma109 commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
nbauma109 commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1303050321

   > They are not used from the existing code base.
   
   Actually there are quite a few places of iteration around getAttributes that I could replace too, to avoid code duplication.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] nbauma109 commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
nbauma109 commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1303092815

   Also any special reason why the parser does not do a recursive parse of inner classes whereas it does it for implemented interfaces ? However changing this would result in double parse for applications that already recurse. Could there be a new method with boolean parameter and the current one would call it with false ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] nbauma109 commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
nbauma109 commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1302324873

   I will update the javadocs with description and since tag. Didn't you see the unit tests ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] garydgregory commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1302718843

   BTW, do you have an actual use case for all these new getters? They are not used from the existing code base.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-bcel] garydgregory commented on pull request #163: added accessors to model and unit tests, javadoc comments

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #163:
URL: https://github.com/apache/commons-bcel/pull/163#issuecomment-1303384880

   > Also any special reason why the parser does not do a recursive parse of inner classes whereas it does it for implemented interfaces ? However changing this would result in double parse for applications that already recurse. Could there be a new parse method with boolean parameter to recurse and the current parse would call it with false ?
   
   ATM, I am mostly concerned with increasing code coverage from tests, not adding APIs. The original author may reveal their intent if they are still watching this component. Changing it now sounds like a bad idea.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org