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

Problem: end is a keyword #26

Closed
c-rack opened this issue Dec 22, 2015 · 3 comments
Closed

Problem: end is a keyword #26

c-rack opened this issue Dec 22, 2015 · 3 comments

Comments

@c-rack
Copy link
Owner

c-rack commented Dec 22, 2015

end is a (reserved?) keyword in Elixir, which we should avoid as a struct field name, see:
https://github.com/elixir-lang/elixir/blob/7b5f4a5842d05bf279b249078fd9051e3dcafa50/lib/elixir/src/elixir_tokenizer.erl#L1037

While this does not cause problems in actual code yet (except broken syntax highlighting), I would strongly advise to change the CIDR struct members start / end to first / last.

This will break backward compatibility, but I think it is better to change it now than later.

@duijf what do you think?

@duijf
Copy link
Collaborator

duijf commented Dec 23, 2015

I'm torn on this.

I'm tempted to say that the code compiles: why change unless we have to?

On the other hand I can see the annoyance in breaking syntax highlighting, and who knows if our code will always be valid? Breaking backwards compatibility now is better than later, especially when we could have seen this comming.

Since we have a fair number of downloads (6k), we do need to issue a deprecation warning first. I'm not sure how to do that with struct keys, though.

@c-rack
Copy link
Owner Author

c-rack commented Dec 24, 2015

I would assume that most users do not directly use the start and end fields of the struct, but rather the match functions in most cases. Nevertheless, let's search for a migration path.

My idea is to add functions that work as accessors to the struct and to treat the struct itself as an opaque object that no project should work directly with. So we would introduce a first/1 and last/1 function and add a warning to README and changelog that direct usage of struct fields is deprecated.

# For example:
x     = "127.0.0.1/24" |> CIDR.parse
first = x |> CIDR.first
last  = x |> CIDR.last

Does it make sense?

@c-rack
Copy link
Owner Author

c-rack commented Dec 25, 2015

Another idea is to add first and last as fields to the struct and duplicate start and end values. That way, new code could use the new struct fields while old code does not need to be updated, yet. We add a deprecation warning to README and changelog finally remove start / end fields on 1.0 release.

c-rack added a commit that referenced this issue Jan 8, 2016
Solution: add additional first/last fields and deprecate start/end

See #26
@c-rack c-rack mentioned this issue Feb 10, 2016
@c-rack c-rack closed this as completed in bb7b44d Mar 6, 2016
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

No branches or pull requests

2 participants