Skip to content

Rewrote implicits section of the tour #746 #1003

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

Closed
wants to merge 3 commits into from
Closed

Rewrote implicits section of the tour #746 #1003

wants to merge 3 commits into from

Conversation

mghildiy
Copy link
Contributor

No description provided.

@mghildiy
Copy link
Contributor Author

I have few questions about this change.
I removed older implicit files(implicit-parameters.md,implicit-conversion.md),introduced new file(implicit.md) with previous-page pointing to self-types and next-page pointing to polymorphic-methods.
But build failed.
Error traces suggests that I need to do similar changes for all language specific directories(ko,es,pl etc).
If yes, then how would I get language translations for implicit.md?

@SethTisue
Copy link
Member

I've answered your question at https://contributors.scala-lang.org/t/multi-language-files/1519

@SethTisue
Copy link
Member

it looks like #1018 will be merged first and you'll need to rebase.

@SethTisue
Copy link
Member

#1018 has been merged.

@SethTisue SethTisue mentioned this pull request Feb 15, 2018
@SethTisue SethTisue added the wip label Feb 15, 2018
@som-snytt
Copy link
Contributor

I was going to fold changes to http://docs.scala-lang.org/tutorials/FAQ/finding-implicits.html on top of this PR, but I'm not sure this is a good foundation. I doubt whether the examples are explanatory.

I also think it's useful to have two pages for params and conversions.

For params, we need an easy typeclass example that is meaningful.

I see the current page links to that faq page; so my task is to inline it. However, I'll do that from head.

@mghildiy
Copy link
Contributor Author

To clarify things more, now that #1018 changes are in master, and as part of this PR I removed implicit-parameters.md, so now I would manually need to take new content from implicit-parameters.md and put them in implicit.md(implicit-parmeters.md is still deleted).

@mghildiy
Copy link
Contributor Author

Can anyone please confirm my last comment?

@SethTisue
Copy link
Member

SethTisue commented Feb 26, 2018

Apologies for the delay. The situation here seems confused and confusing, so probably everyone is (like me) hesitating to comment because it's hard to even understand what's going on.

In particular, I've lost track of what the benefit is supposed to be of merging the two sections into one...? Is there even a consensus on making that change, or was it just an idea Travis had? It doesn't seem like a clear improvement to me. I'm not strongly against, either... I just don't get it. Maybe the benefit just needs to be explained to me.

I'm not sure what other improvements might remain to be incorporated.

@mghildiy
Copy link
Contributor Author

I think it would be better if we close this PR.Instead, I would prefer to first really understand what latest changes in implicit-parameters.md bring to table,and then if merging of implicit files into one still makes sense, we can go further.

@SethTisue
Copy link
Member

@mghildiy okay — thanks for working on this, sorry about the overlap in work that ended up happening

@SethTisue SethTisue closed this Feb 27, 2018
@mghildiy mghildiy deleted the fix746 branch February 28, 2018 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants