search

janhommes / o.js

o.js - client side oData lib.
https://janhommes.github.io/o.js/example/
MIT License
241 stars 58 forks source link

Headers were not included on batch items. #107

Closed melgish closed 4 years ago

melgish commented 4 years ago

I discovered that batch processing was not actually headers to the individual requests.

Updated getHeaders to correct the issue.

janhommes commented 4 years ago

lgtm. Just wondering if we should keep the inline comments. They help me to understand the PR, but I guess at some point we will wonder why we put them there.

melgish commented 4 years ago

Well that's up to you. I think having them explain 'why' a function is written one way vs another is useful. Keeps me from changing it back 2 days later :-) I also think anything that makes the code easier to understand encourages participation in the development.

janhommes commented 4 years ago

yeah, discussable. I guess when code changes often they are scared that the inline comments go out of sync. But as this code doesn't change that often I guess it is fine.

Will merge.