Skip to content

Remove isOtherLowercase and isOtherUppercase from the Character models #35

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

Merged
merged 3 commits into from
Sep 20, 2021

Conversation

NlightNFotis
Copy link
Contributor

These were removed in upstream commit https://github.com/openjdk/jdk/pull/2846/files
and are now creating problems when trying to build the models library with openJDK-17.

@NlightNFotis NlightNFotis self-assigned this Sep 17, 2021
Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

That's a legitmate simplification as the called method has never been modelled.
To be coherent with other changes to JDK files, please comment out the code instead of removing it and put a comment above it that explains why the code has been commented out.

@NlightNFotis NlightNFotis force-pushed the remove-character-methods branch from c0d3fff to 5edc278 Compare September 19, 2021 21:21
@NlightNFotis
Copy link
Contributor Author

Hello @peterschrammel.

I have implemented the changes you suggested.

I'm assuming Travis is dead (or our config is super-broken); does this mean you have to merge manually?

@peterschrammel
Copy link
Member

Can you add a simple github actions configuration to replace it?

@NlightNFotis
Copy link
Contributor Author

Hi @peterschrammel, yes, absolutely.

I will spin one and rebase the current PR on top of the branch that contains it.

For now it's using a simple platform array (`ubuntu-latest`, Java 16
JDK by Adoptium), but it can be enhanced later to feature support for
multiple platforms or JDK versions.
It has gone stale over time and blocks PRs against Models Library
from getting merged. Additionally, it has been substituted by a
Github Action that has the same effect.
… models.

These were removed in upstream commit https://github.com/openjdk/jdk/pull/2846/files
and are now creating problems when trying to build the models library with openJDK-17.
@NlightNFotis NlightNFotis force-pushed the remove-character-methods branch from 5edc278 to a912257 Compare September 20, 2021 09:23
@NlightNFotis
Copy link
Contributor Author

Okay, @peterschrammel, a basic config for CI has been added.

We could expand it to a more featureful array of platforms/environments, but it can also be done as a later step. What would be your preference?

Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

One platform is sufficient for now.

@NlightNFotis
Copy link
Contributor Author

Alright @peterschrammel.

The travis configuration will need to be removed from the Repository's settings as a required check as well, I believe, and then I should be able to merge.

@peterschrammel
Copy link
Member

Should be ready now.

@NlightNFotis NlightNFotis merged commit 012cdc2 into master Sep 20, 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.

2 participants