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

Rust deployment #790

Merged
merged 10 commits into from
Jul 13, 2016
Merged

Rust deployment #790

merged 10 commits into from
Jul 13, 2016

Conversation

johnnyman727
Copy link
Contributor

No description provided.

@cflewis
Copy link

cflewis commented Jul 6, 2016

LGTM

1 similar comment
@tikurahul
Copy link
Contributor

LGTM

@badboy
Copy link
Contributor

badboy commented Jul 6, 2016

I took a look at the code and also ran it.
I had to change a previously mentioned thing to make it work on Node v6:

diff --git i/lib/tessel/deployment/rust.js w/lib/tessel/deployment/rust.js
-    var details = exportables.meta.checkConfiguration(opts.target);
+    var details = exportables.meta.checkConfiguration(opts.target, null, opts.resolvedEntryPoint);

I know v6 might not be officially supported. But if that is the only thing holding it back, can it be added?

@@ -203,8 +203,13 @@ makeCommand('run')
default: false,
help: 'Deploy a project containing all files within, including those not used by the program, excluding any files matched by non-negated rules in .tesselignore and including any files matched by rules in .tesselinclude. Program is started from specified file.'
})
.option('rustCC', {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to suggest making this rustcc, so it's easier for end users to type

Copy link
Contributor

Choose a reason for hiding this comment

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

or possibly rust-cc? Unix command line options are almost always in kebab-case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm good with rust-cc. That's a little more intuitive to me.

Copy link
Contributor

@rwaldron rwaldron Jul 6, 2016

Choose a reason for hiding this comment

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

¯_(ツ)_/¯

Unix command line options are almost always in kebab-case

When they are distinctly two words, yes... but "cc" is an abbreviation right? I'm assuming "cross compiler"? That's two words, so by this logic, you want "rust-c-c" which is just cruel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go with rustcc because it's shortest and, let's be honest, most users are very unlikely to go through the trouble of setting up their own cross compilers. It's pretty much just a developer flag.

@rwaldron
Copy link
Contributor

rwaldron commented Jul 6, 2016

I've just landed #787, this will need to be rebased.

@rwaldron
Copy link
Contributor

rwaldron commented Jul 7, 2016

Needs to be rebased.

@@ -1,3 +1,5 @@
var javascript = require('../../lib/tessel/deployment/javascript');
Copy link
Contributor

Choose a reason for hiding this comment

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

This object is already globally available in this file as deployment.js (same thing for deployment.rs and deployment.py)

@johnnyman727 johnnyman727 force-pushed the jon-rust-deploy-3 branch 3 times, most recently from abc5eef to dbd6663 Compare July 13, 2016 12:34
@johnnyman727
Copy link
Contributor Author

Cleaned up commits and addressed PR comments. I'm going to work on getting the cross compilation server deployed on digital ocean, add the permanent IP here, and then merge.

@johnnyman727
Copy link
Contributor Author

Smoke test for anyone that wants to try this:

mkdir rust-fun; cd rust-fun
t2 init --lang=rust
t2 run Cargo.toml

Watch the LEDs blink to your heart's content.

@johnnyman727 johnnyman727 merged commit 6f4b42b into master Jul 13, 2016
@johnnyman727 johnnyman727 deleted the jon-rust-deploy-3 branch July 13, 2016 19:18
@johnnyman727
Copy link
Contributor Author

👏

@cflewis
Copy link

cflewis commented Jul 13, 2016

Sweet! Awesome work Jon! 🤗

On Wed, Jul 13, 2016, 12:18 PM Jon notifications@github.com wrote:

👏


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#790 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AACqvEIxeXENae9VPRPV2kkfT95bL_5nks5qVTofgaJpZM4JFpPw
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants