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

feat: add deprecation meta #1136

Merged
merged 4 commits into from
Jan 26, 2021
Merged

Conversation

hellerve
Copy link
Member

This PR adds the meta form deprecated. It will be rendered in HTML (see the screenshot below) and in the REPL, like so:

> :i =>
=> : Macro
  Defined at line 27, column 3 in '/Users/veitheller/Documents/Code/Github/carp/carp/core/ControlMacros.carp'
  Deprecated: true

Bildschirmfoto 2021-01-18 um 13 46 44

Cheers

@hellerve hellerve requested a review from a team January 18, 2021 12:48
@jacereda
Copy link
Contributor

I would prefer "Deprecated" instead of "Deprecated: true", I guess that's also a problem now for hidden and friends.

@TimDeve
Copy link
Contributor

TimDeve commented Jan 18, 2021

I would make an explanation of the deprecation mandatory, personally.

@hellerve
Copy link
Member Author

I would prefer "Deprecated" instead of "Deprecated: true", I guess that's also a problem now for hidden and friends.

I agree! I went the extra mile and fixed this for all the boolean values we work with.

@hellerve
Copy link
Member Author

I would make an explanation of the deprecation mandatory, personally.

What would go in the explanation? What to use instead? Why it was deprecated?

@TimDeve
Copy link
Contributor

TimDeve commented Jan 18, 2021

Yeah, I feel that a deprecated note without any guidance is not very useful to the end user.

@eriksvedang
Copy link
Collaborator

Maybe any non-nil value means it's deprecated. Would enable this:

Deprecated: Replaced by ->

@hellerve
Copy link
Member Author

deprecated now receives an optional string argument that will be rendered if it is provided.

Bildschirmfoto 2021-01-19 um 09 45 58

This code was used to generated the above:

(deprecated example)
(defn example [] 1)

(deprecated another-example "was deprecated in favor of nothing. Have fun!")
(defn another-example [] 1)

I think this is the best compromise.

@eriksvedang
Copy link
Collaborator

Looks great, thanks for pleasing us all!

@eriksvedang
Copy link
Collaborator

Huh, I thought I merged this (sorry for dragging my feet). Has conflicts now, will merge when fixed.

@hellerve
Copy link
Member Author

Fixed.

@eriksvedang eriksvedang merged commit 384c419 into carp-lang:master Jan 26, 2021
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

4 participants