Skip to content

Fail fast on language keywords in method names#1235

Merged
raboof merged 2 commits intoakka:masterfrom
andreaTP:failfast-on-keywords
Jan 13, 2021
Merged

Fail fast on language keywords in method names#1235
raboof merged 2 commits intoakka:masterfrom
andreaTP:failfast-on-keywords

Conversation

@andreaTP
Copy link
Copy Markdown
Contributor

References #239

Tested with scripted tests since I haven't found a more suitable place.
This can be a first proposal that provides the user a better error message (it's going to fail the build anyhow).
We can, later on, decide to backtick those keywords in Scala.

@akka-ci
Copy link
Copy Markdown

akka-ci commented Jan 11, 2021

Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK ΤO ΤESΤ' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged.

For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask!

@raboof
Copy link
Copy Markdown
Contributor

raboof commented Jan 11, 2021

OK TO TEST

Copy link
Copy Markdown
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

I agree, nice improvement already!

@raboof
Copy link
Copy Markdown
Contributor

raboof commented Jan 11, 2021

We can, later on, decide to backtick those keywords in Scala

Oh, cool, this is already in progress in #1236

@andreaTP
Copy link
Copy Markdown
Contributor Author

andreaTP commented Jan 11, 2021

Minor issue:
Travis keeps failing because my fork master branch is pretty old and sbt get a previousStableVersion that doesn't exists.
Ref.
https://github.com/akka/akka-grpc/blob/master/build.sbt#L54

Fixed by syncing my fork master branch with origin/master.

@andreaTP
Copy link
Copy Markdown
Contributor Author

Is Jenkins stuck?

@raboof
Copy link
Copy Markdown
Contributor

raboof commented Jan 11, 2021

OK TO TEST

import Method._

require(
!ReservedWords.contains(name),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All scala related fail-fast logic should be removed right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll rebase after #1236 get merged.

@andreaTP andreaTP force-pushed the failfast-on-keywords branch from 7c7e529 to 5ca22b3 Compare January 13, 2021 11:46
@andreaTP
Copy link
Copy Markdown
Contributor Author

rebased on latest master.

Copy link
Copy Markdown
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Thanks for the rebase!

@raboof raboof merged commit 8e65529 into akka:master Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants