Skip to content
This repository was archived by the owner on Jan 1, 2026. It is now read-only.

Conversation

@baphled
Copy link
Contributor

@baphled baphled commented Dec 4, 2019

This PR introduces PlatformIO and TravisCI. To to help with making sure that this library compiles against various boards.

I initially had it built against 1.4, as I noticed an issues with getting 1.5 to work with one or two of my other projects.

So I figured it would be beneficial to add some automation to this project.

@baphled
Copy link
Contributor Author

baphled commented Dec 4, 2019

@nrwiersma It'd be cool to get your thoughts on this. I'm more than happy to tweak it as required :)

@nrwiersma
Copy link
Owner

Hi,

Thanks, this seems like a nice idea and something i would interested in seeing. Couple questions. Are empty folders with just READMEs really needed? Is the main.cpp really needed?

Thanks,
Nick

@nrwiersma
Copy link
Owner

btw I have turned Travis on for this repo, so we should see it run on the next push of this PR.

@cgmckeever
Copy link
Collaborator

@baphled Were there issues with the code in the new release that need to be addressed? Or were there items on your side that fixed the issue?

@baphled
Copy link
Contributor Author

baphled commented Dec 6, 2019

Hi,

Thanks, this seems like a nice idea and something i would interested in seeing. Couple questions. Are empty folders with just READMEs really needed? Is the main.cpp really needed?

Thanks,
Nick

@nrwiersma mine.cpp is just required so that we can build the library. If we remove it we won't get the library to build. Although someone else, with more knowledge of platformio may be able to correct me though.

We could move the extra classes out of ConfigManager.h into the lib directory, for example. Which will help us when it comes to writing tests later.

They're really required to get a test build running though, so more than happy to remove them.

@baphled
Copy link
Contributor Author

baphled commented Dec 6, 2019

@baphled Were there issues with the code in the new release that need to be addressed? Or were there items on your side that fixed the issue?

@cgmckeever There was an issue of compiling against the ESP32 but we could resolve that later. It's easy enough to add and remove.

@baphled
Copy link
Contributor Author

baphled commented Dec 6, 2019

@nrwiersma I've created a POC for keeping the lib directory, the links below.

If you're happy with that I'll add this PR here also, once we've got this one wrapped up.

boodah-consulting#1

@nrwiersma
Copy link
Owner

I have been buggering around with this in a branch, seeing if we can reduce the footprint. This is what I came up with: https://github.com/nrwiersma/ConfigManager/tree/travis-test. WDYT?

I found some issues which i fixed at the same time.

@baphled
Copy link
Contributor Author

baphled commented Dec 6, 2019

Ah, nice, that looks good to me.

Copy link
Owner

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the contribution.

@nrwiersma nrwiersma merged commit c3b1126 into nrwiersma:master Dec 11, 2019
baphled added a commit to boodah-consulting/ConfigManager that referenced this pull request May 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants