Skip to content
This repository has been archived by the owner on Nov 11, 2020. It is now read-only.

Fix crash. #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix crash. #16

wants to merge 1 commit into from

Conversation

sodul
Copy link

@sodul sodul commented Nov 8, 2012

I'm not sure why %d is not working but switching to %s made the tool
report numbers properly without breaking.

I'm not sure why %d is not working but switching to %s made the tool
report numbers properly without breaking.
@thomasf
Copy link

thomasf commented Nov 8, 2012

it was probably something like this #7 (comment)

@thomasf
Copy link

thomasf commented Nov 8, 2012

Or did the error actually include a string?

@sodul
Copy link
Author

sodul commented Nov 8, 2012

Yes, that is the same issue, the output after setting %s showed numbers and not text strings, so my patch should resolve the issue.

Sent from my iPhone

On Nov 8, 2012, at 11:17, Thomas Frössman [email protected] wrote:

Or did the error actually include a string?


Reply to this email directly or view it on GitHub.

@thomasf
Copy link

thomasf commented Nov 8, 2012

But dynamic configuration is allowed, instead the linter should ignore those lines since it cannot understand what they really represent.

@sodul
Copy link
Author

sodul commented Nov 9, 2012

The proper fix is probably to cast the values to int() before passing them.

Sent from my iPhone

On Nov 8, 2012, at 12:27, Thomas Frössman [email protected] wrote:

But text strings are allowed, instead the linter should ignore those lines since it cannot understand what they really represent.


Reply to this email directly or view it on GitHub.

@thomasf
Copy link

thomasf commented Nov 9, 2012

I still belive they should be silently ignored since the linter cannot know if it should warn or not since it cannot execute the environment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants