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

Spelling #2381

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Spelling #2381

wants to merge 17 commits into from

Conversation

jsoref
Copy link

@jsoref jsoref commented Jul 28, 2022

This PR corrects misspellings identified by the check-spelling action.

The misspellings have been reported at jsoref@ecfa03a#commitcomment-79693156

The action reports that the changes in this PR would make it happy: jsoref@f440cae

Note: this PR does not include the action. If you're interested in running a spell check on every PR and push, that can be offered separately.

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref jsoref requested a review from xuwei-k as a code owner July 28, 2022 22:57
Copy link
Author

@jsoref jsoref left a comment

Choose a reason for hiding this comment

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

Most changes were automatically suggested by Google Sheets. All fault mine.

def compose[G[_]](implicit G0: Apply[G]): Apply[λ[α => F[G[α]]]] =
new CompositionApply[F, G] {
implicit def F = self
implicit def G = G0
}

/**The product of Applys `F` and `G`, `[x](F[x], G[x]])`, is a Apply */
/**The product of `Apply`s `F` and `G`, `[x](F[x], G[x]])`, is a Apply */
Copy link
Author

Choose a reason for hiding this comment

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

This seems to be a reasonable match to the use of Apply on line 60

@@ -7,7 +7,7 @@ package scalaz
*
* Day convolution is a special form of Functor multiplication.
* In monoidal category of endofunctors Applicative is a monoid object when Day covolution is used as tensor.
* If we use Functor composition as tensor then then monoid form a Monad instead of Applicative.
* If we use Functor composition as tensor then the monoid form a Monad instead of Applicative.
Copy link
Author

Choose a reason for hiding this comment

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

I'm not at all sure about this.

@@ -7,7 +7,7 @@ package scalaz
* A queue that allows items to be put onto either the front (cons)
* or the back (snoc) of the queue in constant time, and constant
* time access to the element at the very front or the very back of
* the queue. Dequeueing an element from either end is constant time
* the queue. Dequeuing an element from either end is constant time
Copy link
Author

Choose a reason for hiding this comment

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

Happy to drop this or anything else.

@@ -538,7 +538,7 @@ object ContravariantCoyonedaUsage {
s.foldMap{case (st, i) => recItem[BinOrd](i, sortTypeBinOrd(st))}

// The drawback here is that I can’t just build a separate stack
// willynilly for `Binfmt'. I have to prove at each step that *the
// willy nilly for `Binfmt'. I have to prove at each step that *the
Copy link
Author

Choose a reason for hiding this comment

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

Google Sheets made this (and most of the other corrections). I probably wouldn't have actively made it, but it's right.

@@ -25,7 +25,7 @@ object KleisliUsage {
Continent("Asia",
List(
Country("India",
List(City("New Dehli"), City("Calcutta"))))))
List(City("New Delhi"), City("Calcutta"))))))
Copy link
Author

Choose a reason for hiding this comment

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

Proper name...

@@ -28,7 +28,7 @@ case object B extends Token

object Token {

implicit val euqalsRef: Equal[Token] = Equal.equalRef
implicit val equalsRef: Equal[Token] = Equal.equalRef
Copy link
Author

Choose a reason for hiding this comment

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

?

@@ -38,7 +38,7 @@ object DListTest extends SpecLite {
def monoid[A] = Monoid[DList[A]]
def monadPlus = MonadPlus[DList]
def alt = Alt[DList]
def bindrec = BindRec[DList]
def bindRec = BindRec[DList]
Copy link
Author

Choose a reason for hiding this comment

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

This change is the scariest (it occurs in three files).

I can't understand how this code could compile in both forms. Based on the surrounding code, this change appears to be correct, but 🤷 .

Copy link
Collaborator

@Atry Atry Feb 10, 2023

Choose a reason for hiding this comment

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

It's just an unused method to test whether BindRec[DList] compiles or not. So any name would be OK.

@@ -49,7 +49,7 @@ object ListTest extends SpecLite {
a.groupWhenM[Id](p).map(_.list.toList).flatten must_===(a)
}

"groupByWhenM[Id] ∀(i,j) | 0<i<resut.len & 0<j<result(i).len: p(result(i)(j), p(result(i)(j+1)) yields true" ! forAll {
"groupByWhenM[Id] ∀(i,j) | 0<i<result.len & 0<j<result(i).len: p(result(i)(j), p(result(i)(j+1)) yields true" ! forAll {
Copy link
Author

Choose a reason for hiding this comment

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

?

Copy link
Collaborator

@Atry Atry left a comment

Choose a reason for hiding this comment

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

We should not rename public API. It would be nice to create a new method with the correct name and deprecate the old typo method.

@@ -409,7 +409,7 @@ object Heap extends HeapInstances {
ins(f, link(f)(t, tp))(ts)
}

def uniqify[A](f: (A, A) => Boolean): Forest[A] => Forest[A] = {
def uniquify[A](f: (A, A) => Boolean): Forest[A] => Forest[A] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a public API. We should keep the original method for backward compatibility.

Copy link
Author

Choose a reason for hiding this comment

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

I've added a deprecated method with the original name that calls this method: https://github.com/scalaz/scalaz/compare/3b0bbbf7de95f0b02288c57aa839462f2deca9e3..561f47ff91cdcc603cf9ac048b831e88bf504730

Does that do what you're looking for?

Are there other methods that need this treatment?

@jsoref jsoref requested review from Atry and removed request for xuwei-k February 9, 2023 21:56
Copy link
Collaborator

@Atry Atry left a comment

Choose a reason for hiding this comment

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

Still contains public API changes

@jsoref jsoref requested a review from Atry February 9, 2023 22:37
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@Atry
Copy link
Collaborator

Atry commented Feb 10, 2023

Looks good to me.
@xuwei-k What do you think?

I am not sure if we could simply rename a public API without deprecation, considering the master branch does not promise backward compatibility.

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

3 participants