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

Provide public access to ConsoleDatasource #278

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AvdLee
Copy link
Collaborator

@AvdLee AvdLee commented Jul 2, 2024

This opens up some of the APIs to allow fetching entities outside of PulseUI. This is great of building custom UIs or a custom Console.

@AvdLee AvdLee added the enhancement New feature or request label Jul 2, 2024
@AvdLee AvdLee requested a review from kean July 2, 2024 14:03
@AvdLee AvdLee self-assigned this Jul 2, 2024
@kean
Copy link
Owner

kean commented Jul 2, 2024

I think I have an idea for how to organize it that could also help me manage the "Pulse for Mac" codebase: move these internal dependencies to a separate module so that they don't "pollute" the PulseUI namespace but are still there if you need them.

@AvdLee
Copy link
Collaborator Author

AvdLee commented Jul 3, 2024

@kean that makes total sense to me. While it's UI related code, they aren't really User Interface definitions. (In theory, you could use these data fetchers for non-UI work)

@kean
Copy link
Owner

kean commented Jul 7, 2024

I made a couple of changes in main to make it easier to contribute. The biggest changes are the removal of the pbxproj file and the CocoaPods support (long overdue), so it's now easy to update the project structure.

I also made an initial attempt to extract these API to a separate module: PulseComponents #280. I don't think it's a sustainable approach. With the amount of internal APIs RocketSim seems to be using, the best option would be to build these new views as part of the PulseUI framework, so that RocketSim could easily benefit from the future updates to PulseUI and the community could benefit from the updates coming from RocketSim.

@AvdLee
Copy link
Collaborator Author

AvdLee commented Jul 8, 2024

@kean makes total sense! Honestly, for me it would be much better to have this inside PulseUI too! I think it makes more sense for the framework to keep these methods private.

Are you going to continue with PulseComponents or close that journey? I rather wait for that to complete before starting my code migration if that makes sense.

@kean
Copy link
Owner

kean commented Jul 8, 2024

Are you going to continue with PulseComponents or close that journey?

It was an experiment to check if it's a viable option, which it is, but I don't think there is value in shipping these public APIs. I also can't commit to supporting these in the future, as I can't say that I'm happy with many of the internal APIs in PulseUI. On the other hand, I'd be more than happy to add more customization options and new high-level views in the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants