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

Partial fix #1023: Port j.u.NavigableMap #1893

Merged
merged 3 commits into from Jan 13, 2021

Conversation

LeeTibbert
Copy link
Contributor

  • This PR was motivated by Issue Missing methods to support scala-java-time #1023 "Missing methods to support
    scala-java-time". It provides one of the methods missing on
    2020-09-13, java.util.NavigableMap.

    That method was used in TzdbZoneRulesProvider.scala. Tread carefully
    when tracing because that file is generated by the separate
    scala-java-time-tzdb plugin and not in the obvious but wrong
    git repository.

  • No tests are provided because j.u.NavigableMap is an abstract
    trait with no default (concrete) methods. Implementations of the
    trait (interface) will provide their own tests.

Documentation:

  • The standard changelog entry is requested.

  • An entry for j.u.NavigableMap was added to docs/lib/javalib.rst.

Testing:

  • Built and tested ("test-all") in debug mode using sbt 1.3.13 on
    X86_64 only . All tests pass.

  • Local copies of the docs were manually regenerated to verify that
    they build after the change of this PR to javalib.rst.

    The build succeeds with no new errors due to this PR. I believe there
    are PRs awaiting merge which fix the errors unrelated to this PR.

    The expected entry shows up in a browser. As expected, the link is
    broken, because its target does not exist until after this PR is
    merged: circularity.

 * This PR was motivated by Issue scala-native#1023 "Missing methods to support
    scala-java-time". It provides one of the methods missing on
    2020-09-13, java.util.NavigableMap.

    That method was used in TzdbZoneRulesProvider.scala.  Tread carefully
    when tracing because that file is generated by the separate
    scala-java-time-tzdb plugin and not in the obvious but wrong
    git repository.

  * No tests are provided because j.u.NavigableMap is an abstract
    trait with no default (concrete) methods. Implementations of the
    trait (interface) will provide their own tests.

Documentation:

  * The standard changelog entry is requested.

  * An entry for j.u.NavigableMap was added to docs/lib/javalib.rst.

Testing:

  * Built and tested ("test-all") in debug mode using sbt 1.3.13 on
    X86_64 only . All tests pass.

  * Local copies of the  docs were manually regenerated to verify that
    they build after the change of this PR to javalib.rst.

    The build succeeds with no new errors due to this PR. I believe there
    are PRs awaiting merge which fix the errors unrelated to this PR.

    The expected entry shows up in a browser.  As expected, the link is
    broken, because its target does not exist until after this PR is
    merged: circularity.
In recently accepted PR scala-native#1899 it was explained to me that the Scala.js
copyright statement need not be retained when porting Scala.js code
to Scala Native.  I presume & believe the same applies here.
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this pull request Dec 13, 2020
-- Reviewer note ---

This PR supersedes PR scala-native#2016.

This PR touches a number of j.u.Map files but not all.

Any change is a concern, but I believe the changes of this PR
have lower risk.  I did not have to change any "inner" implementation
data types.

Once this PR settles, conditions are ready for a PR which
ports Scala.js CollectionsOnMaps.  That will touch a few
more Map files.  Pending PR scala-native#1893 will probably be folded
into and superseded by the CollectionsOnMaps PR.

-- PR note ---
We port concrete classes which test the Java default methods in j.u.Map.scala.
The tests expect the more competent Scala.js code, so a number
of javalib Map implementation files are also ported. These entirely
replace the existing Scala Native code of the same name.
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this pull request Dec 13, 2020
-- Reviewer note ---
Restart after apparently on unrelated CI failure, rest of tests.

This PR supersedes PR scala-native#2016.

This PR touches a number of j.u.Map files but not all.

Any change is a concern, but I believe the changes of this PR
have lower risk.  I did not have to change any "inner" implementation
data types.

Once this PR settles, conditions are ready for a PR which
ports Scala.js CollectionsOnMaps.  That will touch a few
more Map files.  Pending PR scala-native#1893 will probably be folded
into and superseded by the CollectionsOnMaps PR.

-- PR note ---
We port concrete classes which test the Java default methods in j.u.Map.scala.
The tests expect the more competent Scala.js code, so a number
of javalib Map implementation files are also ported. These entirely
replace the existing Scala Native code of the same name.
@WojciechMazur WojciechMazur merged commit 82efe57 into scala-native:master Jan 13, 2021
ekrich pushed a commit to ekrich/scala-native that referenced this pull request May 21, 2021
* Partial fix scala-native#1023: Port j.u.NavigableMap

 * This PR was motivated by Issue scala-native#1023 "Missing methods to support
    scala-java-time". It provides one of the methods missing on
    2020-09-13, java.util.NavigableMap.

    That method was used in TzdbZoneRulesProvider.scala.  Tread carefully
    when tracing because that file is generated by the separate
    scala-java-time-tzdb plugin and not in the obvious but wrong
    git repository.

  * No tests are provided because j.u.NavigableMap is an abstract
    trait with no default (concrete) methods. Implementations of the
    trait (interface) will provide their own tests.

Documentation:

  * The standard changelog entry is requested.

  * An entry for j.u.NavigableMap was added to docs/lib/javalib.rst.

Testing:

  * Built and tested ("test-all") in debug mode using sbt 1.3.13 on
    X86_64 only . All tests pass.

  * Local copies of the  docs were manually regenerated to verify that
    they build after the change of this PR to javalib.rst.

    The build succeeds with no new errors due to this PR. I believe there
    are PRs awaiting merge which fix the errors unrelated to this PR.

    The expected entry shows up in a browser.  As expected, the link is
    broken, because its target does not exist until after this PR is
    merged: circularity.

* Remove Scala.js copyright, as directed in another PR

In recently accepted PR scala-native#1899 it was explained to me that the Scala.js
copyright statement need not be retained when porting Scala.js code
to Scala Native.  I presume & believe the same applies here.

* Fix typo as an excuse to rebase to ensure PR builds today.
WojciechMazur pushed a commit to WojciechMazur/scala-native that referenced this pull request Aug 25, 2021
* Partial fix scala-native#1023: Port j.u.NavigableMap

 * This PR was motivated by Issue scala-native#1023 "Missing methods to support
    scala-java-time". It provides one of the methods missing on
    2020-09-13, java.util.NavigableMap.

    That method was used in TzdbZoneRulesProvider.scala.  Tread carefully
    when tracing because that file is generated by the separate
    scala-java-time-tzdb plugin and not in the obvious but wrong
    git repository.

  * No tests are provided because j.u.NavigableMap is an abstract
    trait with no default (concrete) methods. Implementations of the
    trait (interface) will provide their own tests.

Documentation:

  * The standard changelog entry is requested.

  * An entry for j.u.NavigableMap was added to docs/lib/javalib.rst.

Testing:

  * Built and tested ("test-all") in debug mode using sbt 1.3.13 on
    X86_64 only . All tests pass.

  * Local copies of the  docs were manually regenerated to verify that
    they build after the change of this PR to javalib.rst.

    The build succeeds with no new errors due to this PR. I believe there
    are PRs awaiting merge which fix the errors unrelated to this PR.

    The expected entry shows up in a browser.  As expected, the link is
    broken, because its target does not exist until after this PR is
    merged: circularity.

* Remove Scala.js copyright, as directed in another PR

In recently accepted PR scala-native#1899 it was explained to me that the Scala.js
copyright statement need not be retained when porting Scala.js code
to Scala Native.  I presume & believe the same applies here.

* Fix typo as an excuse to rebase to ensure PR builds today.
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

2 participants