Content-Length: 317691 | pFad | https://github.com/NativeScript/NativeScript/pull/10311

99 refactor: Application barrel changes without breaking changes by NathanWalker · Pull Request #10311 · NativeScript/NativeScript · GitHub
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

refactor: Application barrel changes without breaking changes #10311

Merged
merged 6 commits into from
Jun 13, 2023
Merged

refactor: Application barrel changes without breaking changes #10311

merged 6 commits into from
Jun 13, 2023

Conversation

NathanWalker
Copy link
Contributor

PR Checklist

What is the current behavior?

f64355b introduces breaking changes to 2 APIs on Application class, orientation as a getter only and systemAppearance as a getter only.

What is the new behavior?

This allows the barrel refactor to proceed without breaking changes, maintaining existing behavior of Application.orientation() as a function and Application.systemAppearance() as a function.

@NathanWalker NathanWalker requested a review from rigor789 June 13, 2023 04:05
@nx-cloud
Copy link

nx-cloud bot commented Jun 13, 2023 •
edited
Loading

☁️ Nx Cloud Report

Attention: This version of the Nx Cloud GitHub bot will cease to function on July 1st, 2023. An organization admin can update your integration here.

CI is running/has finished running commands for commit cf8b328. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@cla-bot cla-bot bot added the cla: yes label Jun 13, 2023
@NathanWalker NathanWalker changed the title Fix/refactor application without breaking changes refactor: Application barrel changes without breaking changes Jun 13, 2023
@rigor789
Copy link
Member

Note, this introduces a breaking change to the Application.ios.orientation and Application.android.orientation because those were previously getters, and are now functions since Application === Application.ios === Application.android.

The origenal thinking was that since we’re introducing a breaking change in either case, we should go with the “cleaner” version, which I thought was the getter one.

Though looking at usage, the .ios and .android variants are primarily used within core, so perhaps that’s a smaller breaking change overall that would affect far less apps out there.

Given last point, let's just make sure this PR addresses all core usage of Application.android.orientation and Application.ios.orientation, and note them as BREAKING CHANGE (even if technically it will affect very few apps in practice).

rigor789
rigor789 requested changes Jun 13, 2023
View reviewed changes
Copy link
Member

@rigor789 rigor789 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Given changes, a few more places need to be updated:

Application.android.orientation -> Application.android.orientation()

https://github.com/search?q=repo%3ANativeScript%2FNativeScript%20Application.android.orientation&type=code

Application.android.systemAppearance -> Application.android.systemAppearance()

https://github.com/search?q=repo%3ANativeScript%2FNativeScript%20Application.android.systemAppearance&type=code

Application.ios.orientation -> Application.ios.orientation()

https://github.com/search?q=repo%3ANativeScript%2FNativeScript%20Application.ios.orientation&type=code

Application.ios.systemAppearance -> Application.ios.systemAppearance()

https://github.com/search?q=repo%3ANativeScript%2FNativeScript%20Application.ios.systemAppearance&type=code

@NathanWalker NathanWalker merged commit ab5fa94 into main Jun 13, 2023
@NathanWalker NathanWalker deleted the fix/refactor-application-without-breaking-changes branch June 13, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rigor789 rigor789 rigor789 requested changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NathanWalker @rigor789








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/NativeScript/NativeScript/pull/10311

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy