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

coffeescript constructors should not call return #1940

Merged
merged 1 commit into from
Mar 5, 2013
Merged

coffeescript constructors should not call return #1940

merged 1 commit into from
Mar 5, 2013

Conversation

ronen
Copy link
Contributor

@ronen ronen commented Feb 25, 2013

coffeescript release 1.5.0 forbids explicit return statements in constructors. When used with the latest coffeescript, these two files now get compilation errors.

But the return statements in these two files were simply returning @, which is superfluous anyway. Deleted them and all seems good.

@seanlinsley
Copy link
Contributor

@ronen, when you fix problems you really should link to the related issue (#1773) to let people know about it! I opened a PR not long after yours because it didn't look like anyone had handled it.

@ronen
Copy link
Contributor Author

ronen commented Feb 27, 2013

@daxter sorry, i didn't know about #1773 or i would certainly have referenced it.

@jviney
Copy link

jviney commented Feb 28, 2013

+1 Fixes coffeescript-1.5.0 compatibility for me.

@seanlinsley
Copy link
Contributor

@chourobin can you please remove that list of gems? It's really polluting this PR. The workaround is to backport coffee-script-source to 1.4.0 in your Gemfile.

@ronen don't sweat it :)

@goosetav goosetav mentioned this pull request Feb 28, 2013
@miclovich
Copy link

Do we need a workaround by downgrading coffee-script-source, we could just remove return in the files of the culprits?

@macfanatic
Copy link
Contributor

@gregbell - Do you have any idea why the return statements were in there in the first place? Appears that everything is working fine for me with the return statements commented out, but want to be a bit cautious on this one.

@seanlinsley
Copy link
Contributor

@macfanatic I haven't looked through the commit history of AA's CoffeeScript files, but just in general it's very convoluted and IMO backwards. I wouldn't be surprised if someone used an automated tool to convert the codebase from JavaScript, or otherwise didn't very well understand CoffeeScript.

macfanatic added a commit that referenced this pull request Mar 5, 2013
coffeescript constructors should not call return, compatible with coffeescript 1.5
@macfanatic macfanatic merged commit 5f7fbea into activeadmin:master Mar 5, 2013
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.

5 participants