Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

METRON-1563 : Base Stellar assign for feature branch #1014

Conversation

ottobackwards
Copy link
Contributor

repackage:
#687

Please sanity check and see that PR

@ottobackwards
Copy link
Contributor Author

@cestella I rebased this on the new feature branch ( after rebasing on the same master ) and I get all of these other commits. I don't know how to get rid of them?

=, +=, -=, *=, /=

This are valid for assignment to variables.  All assignments return the value assigned,
as other scripting languages do.

VariableResolver extended with an update() method, though it may not be supported
by all resolvers, this gives the ability to update variables and save between calls.

The shell was updated for this, such that assignment calls

More testing need and doc but I wanted to get it out there.
This is dependent on PR 686
fixes for handling validate with null values
@ottobackwards
Copy link
Contributor Author

ok, @cestella I have fixed up the feature branch and this pr so it is clean

@cestella
Copy link
Member

Should assignment be = or :=? I'm not super opinionated other than we have been using := and we'd break existing installations if we move to =, right?

@nickwallen
Copy link
Contributor

Should assignment be = or :=?

Ultimately, I'd like to see us move to supporting = as that is just way more intuitive for most people. Maybe that means supporting both = and := for some period of time?

It looks like StellarAssignment was not altered here, so maybe both are supported in this PR? Is that what you are thinking @ottobackwards ?

I am not sure how this PR impacts all the places where Stellar is used and assignment might already occur; field transforms, enrichment, REPL, profiler, etc. There is some other way in which assignment is handled in at least some of these places. I imagine that we would need to update these to use the new mechanism here? Or maybe that is left to a future PR?

@nickwallen
Copy link
Contributor

I am not sure how this PR impacts all the places where Stellar is used and assignment .. Or maybe that is left to a future PR?

Sorry, its a little hard to digest that old PR, but I think from this comment, @ottobackwards is saying that all the other touch points would be updated in a future PR on this feature branch.

@cestella
Copy link
Member

Ultimately, I'd like to see us move to supporting = as that is just way more intuitive for most people. Maybe that means supporting both = and := for some period of time?

In my opinion we should support both for at least a release.

@cestella
Copy link
Member

Do we want to expand assignment to := as part of this PR or make it a follow-on in this feature branch? I'm ok with either way.

@ottobackwards
Copy link
Contributor Author

Can I ask how we do ++, --, += etc with the := notation?

@JonZeolla
Copy link
Member

I wonder if it's good enough just to keep := and = around, but not have that affect the format of ++, +=, etc.

@ottobackwards
Copy link
Contributor Author

I'll expand the assignment to include := in literal stellar, maybe that will bump some review

@ottobackwards
Copy link
Contributor Author

ok, I updated the feature base to track master, and pulled that into this.
I also added support for := @cestella

@cestella
Copy link
Member

Ok, this looks good. +1 to go to the feature branch

@ottobackwards
Copy link
Contributor Author

This is in, not sure why it isn't closed

@justinleet
Copy link
Contributor

@ottobackwards If it's a feature branch, the PR on Github doesn't automatically close. It only happens for merges into a master. It's a bit annoying, but I'm not sure there's a whole lot to do about it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants