Skip to content

Too many warnings! #896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 12, 2017
Merged

Too many warnings! #896

merged 4 commits into from
May 12, 2017

Conversation

rfschtkt
Copy link
Contributor

@rfschtkt rfschtkt commented May 9, 2017

Important warnings warrant attention because they may indicate real problems.
Unimportant warnings warrant attention because they obscure the important warnings.

Unfortunately for me the nicest example of the former category, where I also immediately had a solution, was already fixed independently...

This replaces earlier attempts, now up to date, on its own branch, and with some revisions.

@amitdo
Copy link
Collaborator

amitdo commented May 9, 2017

Warnings are people too!

I see that you changed the title this time... :-)

@stweil
Copy link
Member

stweil commented May 9, 2017

I suggest to make smaller pull requests which focus on closely related problems. That would make reviewing and pulling them much easier. Example: Make a pull request for all changes related to NumFeatureSets.

For that example, the decltype(CharDesc->NumFeatureSets) i will work. Nevertheless I personally prefer the much simpler uinT32 i. Even better because we want to use POSIX instead of Tesseract data types: uint32_t i.

@rfschtkt rfschtkt force-pushed the toomanywarnings branch 2 times, most recently from 75243f9 to edc5f07 Compare May 9, 2017 08:53
@rfschtkt
Copy link
Contributor Author

rfschtkt commented May 9, 2017

You mean like the diminutive "Use POSIX data types and macros" with only 21 commits and 243 files changed?

Yeah, at first I thought typedecl() would be less unpopular than a public GenericVector::unsigned_size(), but I went a bit overboard with it, and I let the cat out of the bag with STRING::unsigned_size() anyway, so... I've amended the pull request without any typedecl(), with public GenericVector::unsigned_size(), but with size_t rather than uintT32/uint32_t.

@stweil
Copy link
Member

stweil commented May 9, 2017

I don't expect that pull request #878 will be applied like that, but yes, it consists of smaller commits which cover limited modifications and which can be applied individually.

@rfschtkt
Copy link
Contributor Author

rfschtkt commented May 9, 2017

Even individual #878 commits are way bigger than this whole pull request (or at least the one I randomly picked). Feel free to split it up further as you like, but I don't see the point: it's already themed by kind of warning, and the larger of the two commits is only +32/−24.

@rfschtkt
Copy link
Contributor Author

rfschtkt commented May 11, 2017

Rebased.

TODO: warnings from "make training"

@zdenop
Copy link
Contributor

zdenop commented May 11, 2017

@rfschtkt: there is merge a conflict in api/pdfrenderer.cpp. Can you fix it?

@rfschtkt
Copy link
Contributor Author

rfschtkt commented May 11, 2017

Warnings from "make training". Rebased.

The remedies were not always obvious, so comments welcome.

@zdenop zdenop merged commit 2b373d1 into tesseract-ocr:master May 12, 2017
@rfschtkt rfschtkt deleted the toomanywarnings branch June 4, 2017 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants