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

Fix Greek Mythology references in compiler package #5786

Merged

Conversation

@janekdb
Copy link
Member

@janekdb janekdb commented Mar 20, 2017

Improve documentation in some packages and correct some typos in these directories,

  • compiler
  • library
@scala-jenkins scala-jenkins added this to the 2.12.2 milestone Mar 20, 2017
@SethTisue
Copy link
Member

@SethTisue SethTisue commented Mar 20, 2017

(CI failure is real)

@janekdb
Copy link
Member Author

@janekdb janekdb commented Mar 20, 2017

Fix this

[error] /home/jenkins/workspace/scala-2.12.x-validate-publish-core/src/compiler/scala/tools/nsc/typechecker/Implicits.scala:363: overriding value runDefinitions in class Typer of type Implicits.this.global.definitions.RunDefinitions;
[error]  value runDefinitions has weaker access privileges; it should not be private
[error]     private val runDefinitions = currentRun.runDefinitions
[error]                 ^
[error] one error found
@janekdb janekdb force-pushed the janekdb:topic/2.12.x-scaladoc-spelling-corrections-3 branch from 9318b9d to e6a44f1 Mar 20, 2017
@janekdb
Copy link
Member Author

@janekdb janekdb commented Mar 20, 2017

Compile error fixed. Should be plain sailing here on in.

image

private val runDefintions = currentRun.runDefinitions
import runDefintions._
private val currentRunRunDefinitions = currentRun.runDefinitions
import currentRunRunDefinitions._

This comment has been minimized.

@Ichoran

Ichoran Mar 20, 2017
Contributor

Ow. That is not a very friendly name. If you're going to use so many letters, maybe at least explain why this whole thing is necessary val stableRunDefsForImport = currentRun.runDefinitions

This comment has been minimized.

@dwijnand

dwijnand Mar 21, 2017
Member

Ya running and ya running. But ya can't run away from yourself.

This comment has been minimized.

@janekdb

janekdb Mar 21, 2017
Author Member

@Ichoran - thanks for thinking about this. Improvement applied!

@@ -1348,7 +1348,7 @@ trait Namers extends MethodSynthesis {

// Add a () parameter section if this overrides some method with () parameters
val vparamSymssOrEmptyParamsFromOverride =
if (overridden != NoSymbol && vparamSymss.isEmpty && overridden.alternatives.exists(_.info.isInstanceOf[MethodType])) ListOfNil // NOTEL must check `.info.isInstanceOf[MethodType]`, not `.isMethod`!
if (overridden != NoSymbol && vparamSymss.isEmpty && overridden.alternatives.exists(_.info.isInstanceOf[MethodType])) ListOfNil // NOTE: Must check `.info.isInstanceOf[MethodType]`, not `.isMethod`!

This comment has been minimized.

@Ichoran

Ichoran Mar 20, 2017
Contributor

Must should be must as it is a sentence fragment. (If we're going to be really pedantic.)

This comment has been minimized.

@janekdb

janekdb Mar 21, 2017
Author Member

Now lowercase matching the majority case,

$ grep -rh -oE '(NOTE:.*)' src/|cut -c1-7|sort|uniq -c
      1 NOTE: `
      1 NOTE: *
      2 NOTE: a
      1 NOTE: A
      1 NOTE: b
     11 NOTE: c
      2 NOTE: C
      1 NOTE: d
      2 NOTE: e
      1 NOTE: E
      3 NOTE: f
      2 NOTE: g
      1 NOTE: h
      1 NOTE: i
      2 NOTE: I
      2 NOTE: m
      2 NOTE: o
      1 NOTE: q
      2 NOTE: r
      1 NOTE: s
      1 NOTE: S
      6 NOTE: t
      1 NOTE: T
      3 NOTE: U
      1 NOTE: V
      4 NOTE: w
      1 NOTE: W
@janekdb janekdb force-pushed the janekdb:topic/2.12.x-scaladoc-spelling-corrections-3 branch 2 times, most recently from cb5ffc3 to 22ac970 Mar 21, 2017
Copy link
Member

@SethTisue SethTisue left a comment

on behalf of the Nicolaas Govert de Bruijn estate, I thank you.

@@ -117,13 +117,13 @@ trait TypeDiagnostics {
*/
final def exampleTuplePattern(names: List[Name]): String = {
val arity = names.length
val varPatterNames: Option[List[String]] = sequence(names map {
val varPatternNames: Option[List[String]] = sequence(names map {

This comment has been minimized.

@paplorinc

paplorinc Apr 4, 2017
Contributor

minor: maybe remove var prefix if it's a val :)

val patternNames

This comment has been minimized.

@janekdb

janekdb Apr 4, 2017
Author Member

In context I think the val is about variables so it's okay to leave as-is,

    val varPatterNames: Option[List[String]] = sequence(names map {
      case name if nme.isVariableName(name) => Some(name.decode)
      case _                                => None
    })

WDYT?

@SethTisue SethTisue modified the milestones: 2.12.3, 2.12.2 Apr 4, 2017
@janekdb janekdb force-pushed the janekdb:topic/2.12.x-scaladoc-spelling-corrections-3 branch 3 times, most recently from a286082 to a23898e Apr 6, 2017
Improve documentation some packages and correct some typos in these directories,

 - compiler
 - library
@janekdb
Copy link
Member Author

@janekdb janekdb commented Apr 10, 2017

Fixed conflicts.

@SethTisue SethTisue modified the milestones: 2.12.2, 2.12.3 Apr 10, 2017
@SethTisue SethTisue merged commit f9514bb into scala:2.12.x Apr 10, 2017
5 checks passed
5 checks passed
cla @janekdb signed the Scala CLA. Thanks!
Details
integrate-ide [4891] SUCCESS. Took 3 s.
Details
validate-main [5547] SUCCESS. Took 100 min.
Details
validate-publish-core [5336] SUCCESS. Took 5 min.
Details
validate-test [4724] SUCCESS. Took 95 min.
Details
@SethTisue
Copy link
Member

@SethTisue SethTisue commented Apr 10, 2017

thanks! looking forward to the PR that straightens out the scalac situation w/r/t qabbalism, the medieval era, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants