Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc: Cleanup and Prep for Arduino IDE #64

Merged
merged 32 commits into from
Feb 15, 2019
Merged

misc: Cleanup and Prep for Arduino IDE #64

merged 32 commits into from
Feb 15, 2019

Conversation

sleepdefic1t
Copy link
Contributor

@sleepdefic1t sleepdefic1t commented Feb 14, 2019

Proposed changes

This PR is cleanup prep for upcoming Arduino IDE branch.
It removes unnecessary files and moves others to a cleaner more Arduino-friendly layout.
It also properly configures ./library.json to export packages correctly for external library managers.

The following has been changed:

  • removed ./appveyor.yml.
  • updated ./library.json package export settings.
  • removed ./test/travis.yml.
  • moved ./docs/ to ./extras in Arduino builds.
  • removed submodule from cmake_example.
  • removed ./CMakeSettings.json.
  • removed uECC_README.md.
  • update keywords.txt.
  • moved ./src/bcl to ./src/lib/
  • moved ./src/rfc6979 to ./src/lib/
  • moved ./src/stl to ./src/lib/
  • updated ARDUINO_IDE.sh script to reflect lib/ changes.
  • updated PIO include and filter paths.

All tests pass locally:

  • macOS - CMake.
  • macOS - PlatformIO.
  • macOS - Arduino IDE

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build (changes that affect the build system)
  • Docs (documentation only changes)
  • Test (adding missing tests or fixing existing tests)
  • Other... Please describe:

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

- list ArduinoJson as a dependency.
- add Arduino sketch path to config.
- fix the 'platformio_example' path.
use newest naming
- bcl
- rfc6979
- stl
- update header includes.
- update CMakeLists.txts.
- update Arduino Script to match changes.
@ghost
Copy link

ghost commented Feb 14, 2019

@air1one @faustbrian - please review this in the next few days. Be sure to explicitly select labels so I know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@ghost ghost requested a review from air1one February 14, 2019 00:49
@sleepdefic1t sleepdefic1t changed the title mics: Cleanup and Prep for Arduino IDE misc: Cleanup and Prep for Arduino IDE Feb 14, 2019
@sleepdefic1t sleepdefic1t changed the title misc: Cleanup and Prep for Arduino IDE [WIP]misc: Cleanup and Prep for Arduino IDE Feb 14, 2019
.gitmodules Show resolved Hide resolved
src/helpers/crypto.cpp Outdated Show resolved Hide resolved
@ghost ghost added the Status: Needs Changes The pull request needs additional changes before it can be merged. label Feb 14, 2019
@ghost
Copy link

ghost commented Feb 14, 2019

@sleepdefic1t Your pull request needs some changes. Please wait for a comment from one of our developers for more information.

@ciband
Copy link
Contributor

ciband commented Feb 14, 2019

Also, why are we moving the docs into extras? I thought the normal pattern was to have docs at the top level directory.

- also update Arduino script to match changes.
@ghost
Copy link

ghost commented Feb 14, 2019

@sleepdefic1t The ci/circleci: build-linux-default job is failing as of bf803204cb9e941b93bcd3622edd10cc40bf8a4e. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@sleepdefic1t
Copy link
Contributor Author

@ciband docs now only moved on Arduino builds 👍

ciband
ciband previously approved these changes Feb 14, 2019
@sleepdefic1t sleepdefic1t changed the title [WIP]misc: Cleanup and Prep for Arduino IDE misc: Cleanup and Prep for Arduino IDE Feb 14, 2019
@sleepdefic1t
Copy link
Contributor Author

This should be good to go now 👍

@faustbrian faustbrian merged commit b9a5798 into ArkEcosystemArchive:master Feb 15, 2019
@ghost ghost removed the Status: Needs Review The issue or pull request needs a review by a developer of the team. label Feb 15, 2019
@sleepdefic1t sleepdefic1t deleted the chore/cleanup branch February 15, 2019 17:50
ciband pushed a commit to ciband/cpp-crypto that referenced this pull request Feb 18, 2019
@ciband
Copy link
Contributor

ciband commented Aug 1, 2019

@sleepdefic1t

This PR broke Visual Studio support on Windows (CMakeSettings.json) as well as Windows CI support via Appveyor (.appveyor.yml).

Do we still want to support Windows?

@sleepdefic1t
Copy link
Contributor Author

@ciband
Yes.
Windows dev is an entirely different beast, but we’d still like to support it.

Is this something you’d be interested in tackling?

@ciband
Copy link
Contributor

ciband commented Aug 2, 2019

I can. We technically still have it as you can do cmake from the command line. That process is exactly the same across all platforms. We broke the nice visual studio integration by deleting the CMakeSettings.json.

For Windows CI, I've been trying to get Ark to setup the Appveyor repo since the beginning of time. The .appveyor.yml file should be all that is needed once that account is setup. Someone from Ark will need to create the account and binding the GItHub repo to it (similar to Circle-CI). There was a long standing PR open on this that I think the stale bot closed.

@sleepdefic1t
Copy link
Contributor Author

@ciband
Appveyor should already be enabled for the C++ SDK’s; so yes, the configs would just need put in place 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: High More than 256 lines changed. Status: Needs Changes The pull request needs additional changes before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants