sparkfun / Weather_Shield

Barometric pressure, temperature, humidity and light sensing weather shield for Arduino.
https://www.sparkfun.com/
Other
69 stars 67 forks source link

Leverage Git Submodules to pull in required libraries. #16

Closed zfields closed 8 years ago

zfields commented 10 years ago

This will address the confusion surrounding the multiple repositories, and bring everything together so it will just work.

zfields commented 10 years ago

Please try this out for yourself, and if I can improve the experience let me know. I think it would be great to have it working this easily.

zfields commented 9 years ago

Your circuit's dead, there's something wrong Can you hear me, Major Tom? ;-)

brennen commented 9 years ago

This may be a correct thing to do.

On the other hand, submodules are terrible and have never created anything but problems for anyone I know, so there's that.

zfields commented 9 years ago

Well, they certainly can be a lot to wrap your head around. However they can also be handy, as they are associated with the last known working commit, and they stay fixed there until modified. So if these are leveraged properly, they will give your users the ability to pull a single repo, compile, deploy and party on your hardware without delay.

zfields commented 9 years ago

What is sparkfun's stance on submodules? Were you speaking for the company when you said submodules were "terrible and never created anything but problems" or was that your personal experience/opinion? Are you guys having a conversation around this, or should I close this pull request?

brennen commented 9 years ago

I'm not speaking for @nseidle or @ToniCorinne, and I'm not in Engineering here. I expect that there will be a conversation there.

My input is just that, while this is certainly the most reasonable and least-likely-to-backfire use case for the feature, I've never had it not burn me. We do avoid them by policy on the software dev side of things here after some bad experiences. It may well be worth trying in this case, especially since there's not really a multi-dev branching workflow for most of these projects.

brennen commented 9 years ago

(And while I'm at it, thank you one way or another for the pull request. It's at minimum a conversation worth having, and we very much appreciate contributions like this. I'm a bit cranky lately, so sorry for coming off like a jerk.)

Jacquot-SFE commented 9 years ago

Please leave it open for a while longer. Toni, our Git Guru, has been AFK at the Open Hardware Summit and Makerfaire Rome.

Brennen is right, this is a conversation that's worth having, but we need to involve the key stakeholders. Toni already has to clean up the Git messes the rest of us make, and I'm not sure whether another sophisticated feature would make that job any easier.

On Thu, Oct 9, 2014 at 1:24 PM, Zachary J. Fields notifications@github.com wrote:

What is sparkfun's stance on submodules? Were you speaking for the company when you said submodules were "terrible and never created anything but problems" or was that your personal experience/opinion? Are you guys having a conversation around this, or should I close this pull request?

— Reply to this email directly or view it on GitHub https://github.com/sparkfun/Weather_Shield/pull/16#issuecomment-58563437 .

zfields commented 9 years ago

Cool, thanks for getting back to me. Sorry to push back so much, but it's been floating for a while, and I just hadn't heard anything one way or the other. I understand the impact of engineering decisions, and it is always wise to consider things fully. I love contributing to cool/fun projects/companies like yours, and I appreciate your candor. Cheers.

nseidle commented 9 years ago

I've never done submodules before so it's a bit scary but I'm not opposed to it.

Toni - you cool with this?

zfields commented 9 years ago

You can fork/clone my repo and monkey with it. That way you have a 0.0% chance of messing up your actual repo. It might be a easy way to play and decide if submodules work for you guys.

ToniCorinne commented 9 years ago

The submodule discussion has gone back and forth in engineering/catalog/IT/production over and over again. It's something I've been wanting to play with to see how feasible it would be to get everyone on the same page, so I'll take a look at this.

No promises this will be a quick or easy fix @zfields, so please keep that in mind. We aren't ignoring this, but adding in/changing major processes like this gets harder to implement the more people who end up working with each repo.

zfields commented 9 years ago

Thank you guys for looping me into the dialogue! I understand the significance, and I think you guys are handling it smashingly. I'm in no rush, I was only concerned before, because I hadn't heard anything; I didn't even know if I was on your radar. Cheers!

zfields commented 8 years ago

I haven't heard anything about this, but I just updated the branch on the off chance you wanted to pull it. ;)

ToniCorinne commented 8 years ago

Sorry for the radio silence on this! We actually decided to go with subtrees as our method of addressing issues like this, due to having too many 'off/exception' cases in our system that don't really work well with submodules. I've been slowly updating all the products as we come across them that need this update, but I will fix this one asap. Thanks again for keeping us on track!

zfields commented 8 years ago

If I can update my commit to match your style and get pulled, I'm happy to do it. I would love to have a commit on your repo! Thanks for the quick reply, Zak

ToniCorinne commented 8 years ago

Hey @zfields - Sorry for the delay again! If you'd like to do the pull request with the subtrees, we'd like it to be in the following format:

For adding each subtree, you'd use the following command: git subtree add -P Libraries/Arduino/[LibraryName] --squash [LibraryRepositoryAddress] master

nseidle commented 8 years ago

Hi zfields - thank you again for your help making this code better. Arduino has diverged enough over the past few years with how a user installs a library that I believe this pull request is no longer relevant. I'm cleaning up the repo and tutorial. If you see anything that could use improvement please let us know!