Skip to content

Conform foreach tparam to majority naming convention#4803

Merged
SethTisue merged 1 commit intoscala:2.11.xfrom
janekdb:2.11.x-conform-foreach-tparam
Oct 27, 2015
Merged

Conform foreach tparam to majority naming convention#4803
SethTisue merged 1 commit intoscala:2.11.xfrom
janekdb:2.11.x-conform-foreach-tparam

Conversation

@janekdb
Copy link
Member

@janekdb janekdb commented Oct 18, 2015

'U' is the common choice for the foreach function result tparam.

This command summarises the naming diversity before and after this change.

$ fgrep -r 'def foreach[' *|cut -f2 -d:|cut -f1 -d'('|tr -s ' '|sed 's/override //g'|sort|uniq -c|sort -nr

Before,

 80  def foreach[U]
  6  def foreach[C]
  6  def foreach[B]
  4  final def foreach[U]
  3  def foreach[S]
  2  inline final def foreach[U]
  2  def foreach[A]
  1  inline final def foreach[specialized
  1  final def foreach[B]
  1  * def foreach[U]
  1  def foreach[Q]
  1  def foreach[D]
  1  def foreach[A,B,U]

After,

 98  def foreach[U]
  5  final def foreach[U]
  2  inline final def foreach[U]
  1  inline final def foreach[specialized
  1  * def foreach[U]
  1  def foreach[A,B,U]

(@ symbols removed.)

@scala-jenkins scala-jenkins added this to the 2.11.8 milestone Oct 18, 2015
@janekdb
Copy link
Member Author

janekdb commented Oct 18, 2015

This was originally part of a larger PR. It started life as #4651.

This commit is mainly about the tparam name used with foreach. There were a number of whitespace edits in the original commit which survive unchanged in this PR.

Review by @adriaanm, @SethTisue, @som-snytt, @Ichoran, @jvican.

@janekdb janekdb mentioned this pull request Oct 18, 2015
33 tasks
@som-snytt
Copy link
Contributor

LGTM

I first noticed this convention in Future for Unit or a value you don't mind discarding. Is there an Abide rule for type params named U that are inferred to be something besides Unit?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you either take this out, or bring aggregate in ParIterableLike.scala in line as well (and update the commit message)? either way. (nbd, but having spotted it, might as well point it out.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the par type params were especially creatively non-conformist, as I recall from when I peeked under that hood/bonnet. It's too bad Dr Odersky didn't ask for straw man proposals for type param conventions for collections. Lets settle the fundamentals first, before building castles in the sky.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot. Done.

'U' is the common choice for the foreach function result tparam.

This command summarises the naming diversity before and after this change.

$ fgrep -r 'def foreach[' *|cut -f2 -d:|cut -f1 -d'('|tr -s ' '|sed 's/override //g'|sort|uniq -c|sort -nr

Before,

     80  def foreach[U]
      6  def foreach[C]
      6  def foreach[B]
      4  final def foreach[U]
      3  def foreach[S]
      2  inline final def foreach[U]
      2  def foreach[A]
      1  inline final def foreach[specialized
      1  final def foreach[B]
      1  * def foreach[U]
      1  def foreach[Q]
      1  def foreach[D]
      1  def foreach[A,B,U]

After,

     98  def foreach[U]
      5  final def foreach[U]
      2  inline final def foreach[U]
      1  inline final def foreach[specialized
      1  * def foreach[U]
      1  def foreach[A,B,U]

(@ symbols removed.)
@janekdb janekdb force-pushed the 2.11.x-conform-foreach-tparam branch from c596533 to 6ed7010 Compare October 21, 2015 21:36
@SethTisue
Copy link
Member

LGTM assuming Jenkins signs off

@janekdb
Copy link
Member Author

janekdb commented Oct 23, 2015

LGTM

@janekdb
Copy link
Member Author

janekdb commented Oct 23, 2015

#4760 can be rebased and taken off on-hold following the merge of this.

SethTisue added a commit that referenced this pull request Oct 27, 2015
Conform foreach tparam to majority naming convention
@SethTisue SethTisue merged commit a24ca7f into scala:2.11.x Oct 27, 2015
@SethTisue
Copy link
Member

thank you Janek!

@janekdb janekdb deleted the 2.11.x-conform-foreach-tparam branch October 27, 2015 12:48
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