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.

Sign up for GitHub

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

Jump to bottom

[BE2] prepare for Scala 3 migration #1113

Closed
wants to merge 6 commits into from
Closed

[BE2] prepare for Scala 3 migration #1113

wants to merge 6 commits into from

Conversation

bbjubjub2494
Copy link
Collaborator

@bbjubjub2494 bbjubjub2494 commented Jun 13, 2022 •
edited
Loading

this is a subset of #1111 that can be applied without actually switching to Scala 3. My hope is proactively doing these changes now will reduce friction when we are ready to switch.

@bbjubjub2494 bbjubjub2494 requested a review from a team as a code owner June 13, 2022 13:51
@bbjubjub2494 bbjubjub2494 force-pushed the work-lourkeur-be2-prepare-scala3 branch from 4fa8ba4 to 5711b1b Compare June 13, 2022 14:03
@bbjubjub2494 bbjubjub2494 mentioned this pull request Jun 14, 2022
Closed
@bbjubjub2494 bbjubjub2494 self-assigned this Jun 14, 2022
@bbjubjub2494 bbjubjub2494 force-pushed the work-lourkeur-be2-prepare-scala3 branch from 5711b1b to 09eafff Compare June 18, 2022 16:30
rombirli
rombirli previously approved these changes Jun 18, 2022
View reviewed changes
Copy link
Contributor

@rombirli rombirli left a comment

Choose a reason for hiding this comment

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

Looks really good overall

...src/test/scala/ch/epfl/pop/model/network/method/message/data/socialMedia/AddChirpSuite.scala
@@ -28,7 +29,7 @@ class AddChirpSuite extends FunSuite with Matchers {
object AddChirpExample {
final val TEXT: String = "text"
final val PARENT_ID: Option[String] = None
final val TIMESTAMP = Timestamp(0)
final val TIMESTAMP: Timestamp = Timestamp(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at some places it was unnecessary to add the typing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I personally don't mind but the "migrate" plugin made that change.

be2-scala/src/test/scala/ch/epfl/pop/model/objects/SignatureSuite.scala
"😀", "OMEGA \u03A9", "\u03A8",
"Non empty can be fully non empty",
"Not false is true")

final val verify_pk = PublicKey(Base64Data.encode(tester.keyPair.getPublicKey))
final val verify_pk: PublicKey = PublicKey(Base64Data.encode(tester.keyPair.getPublicKey))
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same

tuomas-p
tuomas-p previously approved these changes Jun 18, 2022
View reviewed changes
Copy link
Contributor

@tuomas-p tuomas-p left a comment

Choose a reason for hiding this comment

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

LGTM, as this is for pure migration purposes, the coverage failure shouldn't be considered relevant imo

@bbjubjub2494
Copy link
Collaborator Author

@jbsv @pierluca I need either of you to force-merge this PR. The coverage check is in the way.

The rationale is that the changes are automated rewrites that are functionally equivalent. They are peppered all over the place so there's no clear behavior that I should go write a test about.

@bbjubjub2494 bbjubjub2494 dismissed stale reviews from tuomas-p and rombirli via 71c7164 June 23, 2022 15:38
@bbjubjub2494 bbjubjub2494 force-pushed the work-lourkeur-be2-prepare-scala3 branch from 09eafff to 71c7164 Compare June 23, 2022 15:38
@bbjubjub2494 bbjubjub2494 force-pushed the work-lourkeur-be2-prepare-scala3 branch 3 times, most recently from 387a32e to 95567f7 Compare July 6, 2022 13:25
@sonarcloud
Copy link

sonarcloud bot commented Jul 6, 2022

[PoP - Be1-Go] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Jul 6, 2022

[PoP - Be2-Scala] SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

31.2% 31.2% Coverage
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Jul 6, 2022

[PoP - Fe2-Android] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Jul 6, 2022

[PoP - Fe1-Web] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@bbjubjub2494 bbjubjub2494 force-pushed the work-lourkeur-be2-prepare-scala3 branch from 95567f7 to 89b93b4 Compare September 1, 2022 11:56
@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2022

[PoP - Be1-Go] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2022

[PoP - Be2-Scala] SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

31.2% 31.2% Coverage
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2022

[PoP - Fe2-Android] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2022

[PoP - Fe1-Web] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@K1li4nL
Copy link
Contributor

K1li4nL commented Feb 25, 2024

Done in #1742

@K1li4nL K1li4nL closed this Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rombirli rombirli rombirli left review comments

@tuomas-p tuomas-p tuomas-p left review comments

@Ajkunas Ajkunas Awaiting requested review from Ajkunas

Assignees

@bbjubjub2494 bbjubjub2494

Labels
be2-scala
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bbjubjub2494 @K1li4nL @rombirli @tuomas-p