search

dlazaro66 / QRCodeReaderView

Modification of ZXING Barcode Scanner project for easy Android QR-Code detection and AR purposes
1.9k stars 491 forks source link

Fix example in readme #113

Closed auchri closed 7 years ago

dlazaro66 commented 7 years ago

I'm sorry, but I decided I'll not merge this PR because it doesn't add any value to the project nor the codebase. The idea behind pull-requests is to improve the current codebase, add new features or solve bugs for example, but I don't consider that change a variable name in the Readme.md will improve anything.

auchri commented 7 years ago

It does improve the current codebase? lol

A broken documentation is actually a bug so my PR "solve bugs ".

auchri commented 7 years ago

You maybe didn't notice that in the current example qrCodeReaderView and mydecoderview where used for the same thing. Of course one of the two didn't exist (at least before your commit https://github.com/dlazaro66/QRCodeReaderView/commit/20fac1fad12f00e5c80162e279635c17ba40bc4b)

dlazaro66 commented 7 years ago

Hey @auchri first, I'm sorry for not merging your PR.

Let me explain you my thoughts about it; IMHO, the README file shouldn't be a copy&paste full working example but a demo about how can you use the library. You have a full example using all the library capabilities on samples directory. It's true that there were two different variable names but I don't see that as a blocker for you to integrate the library in your project as you'll have your own code structure (maybe you're not going to call that field myQrDecoderView , right?).

Anyway, I'm sorry if you have the sensation I was not aware of that and I "copied" your commit. As you can see here, other people pointed that before and I gave them the same response: https://github.com/dlazaro66/QRCodeReaderView/pull/104 https://github.com/dlazaro66/QRCodeReaderView/pull/91

It's my fault I forgot to do it before (maybe open an issue would have been great). This time, I needed to modify the Readme and release a new version, and as you also pointed this as a problem too, I decided to change that variable name in the commit you mention. In my opinion, as said, this slight readme modification is not enough reason to make a PR as, again imo, it doesn't add value to the codebase.

Finally, if you think you deserve the recognition of this work or this commit and appear on the contributor's list of the project just say it; I'd revert my commit and reopen this PR in order to be merged.

Sorry for the inconveniences again, and thanks for your time.