Skip to content
This repository has been archived by the owner on Jun 2, 2020. It is now read-only.

Last two crates off nightly #131

Merged
merged 2 commits into from Feb 5, 2019
Merged

Conversation

rylev
Copy link
Collaborator

@rylev rylev commented Feb 1, 2019

No description provided.

@cla-bot cla-bot bot added the cla-signed The CLA has been signed by the contributor. label Feb 1, 2019
@peterhuene
Copy link
Owner

peterhuene commented Feb 1, 2019

I think we need to feature-gate the change to remove Diagnostic as without it we get poor error messages from the proc macros.

Before this change:

error: cannot bind to 'nope' because it is not a binding parameter of the function
 --> examples/http/src/functions/greet.rs:5:18
  |
5 | #[binding(name = "nope", auth_level = "anonymous")]
  |                  ^^^^^^

With this change:

error: custom attribute panicked
 --> examples/http/src/functions/greet.rs:4:1
  |
4 | #[func]
  | ^^^^^^^
  |
  = help: message: cannot bind to 'nope' because it is not a binding parameter of the function

@rylev
Copy link
Collaborator Author

rylev commented Feb 1, 2019

@peterhuene The code changes should be well set up to handle feature gating. I tried using cfg-if to handle auto switching between nightly and stable but didn't get it working.

I'll get something working before we merge

@rylev
Copy link
Collaborator Author

rylev commented Feb 4, 2019

@peterhuene This is ready to go I believe. I think the next thing to do is to change the generated cargo project to have a unstable or diagnostics feature which sets the unstable feature on azure-functions and then change the docker file to run cargo with this feature set. I can do that in a separate PR.

@peterhuene
Copy link
Owner

Thanks @rylev! I'll review this in-depth tonight and hopefully get it merged. I can update the Dockerfile template to enable the unstable feature for cargo func new-app. I plan on finishing the cargo func new command this week, at which point I think it warrants a new release with all these changes.

@peterhuene peterhuene self-assigned this Feb 4, 2019
@peterhuene peterhuene added this to the 0.4.0 milestone Feb 4, 2019
@peterhuene
Copy link
Owner

@rylev can you rebase and possibly squash? I should have held off on merging in the dependency updates, sorry about that.

),
})
.emit();
let foo = match s.value().as_ref() {
Copy link
Owner

Choose a reason for hiding this comment

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

foo is a placeholder name that generates a warning for clippy.

Perhaps message instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops. How embarrassing!

Copy link
Owner

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

Looks great, just a few feedback changes to make. Awesome work!

@peterhuene peterhuene merged commit 37f534f into peterhuene:master Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed The CLA has been signed by the contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants