hamishcunningham / fishy-wifi

Scripts, notes and the odd subaquatic gizmo for the ESP8266 and what-have-you.
GNU Affero General Public License v3.0
25 stars 13 forks source link

ArduinoIFTTT library for creating Adafruit feeds, triggers, and triggering IFTTT WebHooks. #23

Open andrisole92 opened 6 years ago

andrisole92 commented 6 years ago

Documentation here.

layerzerolabs commented 6 years ago

I know you said you might be out of time, but anyway here's some feedback... Small things - can you give this pull request a better title? These sort of labels help humans keep track - like variable names, failedSends is a better variable name than X even though the computer doesn't care.... Also I think you should be clear if adapting code from other libraries - probably ought to include their notice as well - if you're just taking a couple of declarations from header, then should probably loose the comments about which boards it suits - esp. as they've probably not been tested by you... And watch out for accidentally committing your credentials to git with code - done it myself but still, replace with placeholders, commit, then you can do a git ignore on the config header...

andrisole92 commented 6 years ago

Yeah, sure @layerzerolabs .

I just wanted to reference the issue.

That is what I wanted to ask about. How to be with extra libraries, that I have been using? I can include links to them.

layerzerolabs commented 6 years ago

I'm hoping @Eroc33 can chip in to correct me, but if the other libraries are on github you can use a git submodule to reference them - minor disadvantage seems to be that the regular git clone for the main code doesn't automatically fetch the submodules ... One alternative would be to add those files in the top level alongside waterelf32 and reference them with "" rather than <>... that solution feels a bit 'dirty' even to me though... I'm sure there must be a better way(tm)? Anyone, anyone - Bueller?

Eroc33 commented 6 years ago

I think the git submodule approach is fairly simple yes (I haven't used them much myself), but I don't think you'd end up with developer having different version of dependencies as there's some sort of commit based versioning for submodules, but this might mean you have to update them (just run some git command, then commit and push as usual I think). However it would presumably mean changing the location the Arduino ide is looking for (is the just the sketchbook path?). Whatever we decide on here I probably should do in pull#25 also.

layerzerolabs commented 6 years ago

Thanks - yes I read more and realised that freezing submodules is the norm and part of the whole point... I see what you mean about sketchbook location - we want to add a library to a directory outside of the repository local copy.... ie where the sketchbook location points...mmm I'll do some searching - this must be common issue. Yes we should nail this for your changes too - and also we've got probably a dozen static copies of libraries that could/should be converted to frozen submodule links once we are confident.

layerzerolabs commented 6 years ago

I think it was this issue of library management being limited on Arduino to the sketchbook location that mean that we set fishy-wifi/ardesp as our sketchbook location... You can see there we already have a single library implemented as a submodule - also the hardware stuff is a submodule because it's enormous and if you have it already downloaded you can just copy or link it across...