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

Add perspective to the community build #10750

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

Katrix
Copy link
Contributor

@Katrix Katrix commented Dec 11, 2020

No idea if I've done this correctly, but let's hope so :P

This will add perspective to the community build. perspective is a generic programming library like shapeless, but taking a different paradigm instead (functional instead of logic based, value level instead of type level). Reason why I thought perspective might fit in well in the community build is that it is probably one of the projects that uses polymorphic function types the most as of now. I've already come across a few bugs in relation to this, which I have already submitted.

No tests for now, as I don't know if there are any other libraries updated to Dotty which includes typeclasses that are easy to test. (Using circe currently for the scala 2 part of the project). Just the fact that it compiles should at least provide some confidence that nothing broke in terms of typechecking or similar which perspective relies upon.

@OlivierBlanvillain
Copy link
Contributor

@Katrix One of the community test build seems to be failing, did you have a change to investigate?

@Katrix
Copy link
Contributor Author

Katrix commented Dec 21, 2020

Ah yes. It's something about syntax. Just need to find some time to get back to this

@smarter smarter assigned Katrix and unassigned anatoliykmetyuk Dec 21, 2020
@Katrix
Copy link
Contributor Author

Katrix commented Dec 26, 2020

Worked fine on local at least now

.gitmodules Outdated
@@ -174,3 +174,6 @@
[submodule "community-build/community-projects/scalatestplus-junit"]
path = community-build/community-projects/scalatestplus-junit
url = https://github.com/dotty-staging/scalatestplus-junit.git
[submodule "community-build/community-projects/perspective"]
path = community-build/community-projects/perspective
url = https://github.com/Katrix/perspective.git
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ok to use the original repo directly instead of a fork? If we don't have permissions to fully manage this repo, we might not be able to fix our tests in case some changes are pushed there without all tests passing for scala 3 or with flaky tests. @smarter ?

Copy link
Member

Choose a reason for hiding this comment

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

Our current policy is to always make a fork in dotty-staging indeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@smarter I could create the fork but it looks like I don't have the right permissions for that

Copy link
Member

Choose a reason for hiding this comment

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

You should have access now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Katrix you can now use

https://github.com/dotty-staging/perspective.git

instead

@Katrix
Copy link
Contributor Author

Katrix commented Feb 3, 2021

Sorry for taking so long with this rebased on top of master and updated the submodule path. The build is currently failing, to me, those errors look like exactly the kind of bugs I thougth having perspective in the community build would help spot.

@griggt
Copy link
Contributor

griggt commented Feb 4, 2021

Seems like some updates may be required due to #10940?

@Katrix
Copy link
Contributor Author

Katrix commented Feb 4, 2021

Alright, updated stuff to the latest nightly version on my repo. How is that now transferred to the dotty-staging repo?

Did notice a strange bit here where I had to reference the extension method I wanted to call from outside the extension block. Seems like in other cases I didn't have to.
https://github.com/Katrix/perspective/blob/dotty/nightly/dotty/perspective/src/main/scala/perspective/DistributiveK.scala#L14

@prolativ
Copy link
Contributor

prolativ commented Feb 4, 2021

@Katrix if you want your project to use a specific branch in the community build you should specify it .gitmodules like this

[submodule "community-build/community-projects/perspective"]
	path = community-build/community-projects/perspective
	url = https://github.com/dotty-staging/perspective.git
	branch = dotty/nightly

I updated this branch inside the fork to the latest revision.
You should also rebase this PR to dotty's latest master

@Katrix Katrix force-pushed the community-build/perspective branch from 9a1773b to 090bfbf Compare February 4, 2021 19:41
@Katrix
Copy link
Contributor Author

Katrix commented Feb 4, 2021

Alright all green now. Should I squash the commits or clean up the history in any way?

Copy link
Contributor

@griggt griggt left a comment

Choose a reason for hiding this comment

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

I think it would be nice to squash the commits. Otherwise, LGTM.

@Katrix Katrix force-pushed the community-build/perspective branch from e69000a to 8a60b77 Compare February 5, 2021 00:16
@Katrix
Copy link
Contributor Author

Katrix commented Feb 5, 2021

Alrighty, all squashed

@griggt griggt assigned anatoliykmetyuk and unassigned Katrix Feb 5, 2021
@prolativ prolativ merged commit 8b1188b into scala:master Feb 5, 2021
@Katrix Katrix deleted the community-build/perspective branch February 5, 2021 08:27
michelou pushed a commit to michelou/dotty that referenced this pull request Feb 5, 2021
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.

6 participants