AstuteSource / chasten

:dizzy: Chasten Uses XML and XPATH to Check a Python Program's AST for Specified Patterns!
https://pypi.org/project/chasten/
GNU General Public License v2.0
7 stars 8 forks source link

fix: patching limits on local checks file #88

Closed simojo closed 9 months ago

simojo commented 9 months ago

This PR patches the limits on the local check file such that they will not pose an obstacle for us in getting our build to pass. It does not change any functionality other than increasing the max value of counts for each check to 300.

boulais01 commented 9 months ago

Hey @simojo, it looks like the checks are still failing on this PR. Is that the expected outcome?

simojo commented 9 months ago

@boulais01 yes, there are two separate issues causing our build to fail. They're both in separate PRs. See PR #87

boulais01 commented 9 months ago

@laurennevill @jnormile @AidanNeeson @tuduun @gkapfham Can we get a look at this and the linked PR(#87 )?

jnormile commented 9 months ago

Could @gkapfham speak to the use of the current maximum numbers? I'd ask as to the intent behind using those specific values before just papering over them with new maximum values.

jnormile commented 9 months ago

@AidanNeeson Were you listening in on our conversation? That's almost exactly what @gkapfham said. 😆

CalebKendra commented 9 months ago

@simojo made a feature that allowed us to fix this at #68

AidanNeeson commented 9 months ago

@AidanNeeson Were you listening in on our conversation? That's almost exactly what @gkapfham said. 😆

I had to make sure I documented his valuable insights for everyone to see! 🤣

simojo commented 9 months ago

Hi @AidanNeeson @jnormile @gkapfham, I believe this is addressed here. Please take time to look this fix over before discussing any further.

laurennevill commented 9 months ago

@gkapfham, to my understanding, this PR would not be necessary if PR #69 is merged successfully. @simojo can comment more on this if I am not correct.

For now, it is not on the priority list.

simojo commented 9 months ago

@gkapfham, to my understanding, this PR would not be necessary if PR #69 is merged successfully. @simojo can comment more on this if I am not correct.

For now, it is not on the priority list.

Yes, we can close this for that reason.