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

feat: Do not display hidden settings #1873

Conversation

ReubenFrankel
Copy link
Contributor

Follow-on from #1872 and related to #1418

Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for meltano-hub ready!

Name Link
🔨 Latest commit 9c07405
🔍 Latest deploy log https://app.netlify.com/sites/meltano-hub/deploys/6717c998364c6c0008eb6242
😎 Deploy Preview https://deploy-preview-1873--meltano-hub.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ReubenFrankel
Copy link
Contributor Author

ReubenFrankel commented Oct 22, 2024

I believe tap-googleads is currently the only plugin with any hidden settings:

https://deploy-preview-1873--meltano-hub.netlify.app/extractors/tap-googleads

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Oct 22, 2024

I'm having second thought on the semantics of hidden.

@ReubenFrankel do you feel like it's more intended for UIs that display (and set) Meltano plugin config? For example, for settings that may not be user-friendly.

So, I wonder if we need a deprecated flag to better reflect situations like this.

@ReubenFrankel
Copy link
Contributor Author

I'm having second thought on the semantics of hidden.

@ReubenFrankel do you feel like it's more intended for UIs that display (and set) Meltano plugin config? For example, for settings that may not be user-friendly.

So, I wonder if we need a deprecated flag to better reflect situations like this.

We use hidden for that exact purpose in our UI, where some settings are hidden deliberately as they should be configured by the system only. I understand where you are coming from though, it's just odd that hidden isn't (I don't think) used anywhere in the CLI or Hub currently - I guess it was a hang-over from the old Meltano UI?

Totally fine by me to drop this, especially since it's just tap-googleads that this would "benefit". Until more plugins are actively deprecating settings, I don't think the flag is super necessary.

@edgarrmondragon
Copy link
Collaborator

@ReubenFrankel let's just keep the deprecation notice in the setting description. We can look at adding a deprecated to Meltano and the Hub plugin schema flag later on.

@edgarrmondragon
Copy link
Collaborator

I guess it was a hang-over from the old Meltano UI?

Yeah, but it also doesn't cost us much keeping it around, and is still useful for anyone trying to implementing any sort of settings UI for Meltano 🙂

@ReubenFrankel
Copy link
Contributor Author

I guess it was a hang-over from the old Meltano UI?

Yeah, but it also doesn't cost us much keeping it around, and is still useful for anyone trying to implementing any sort of settings UI for Meltano 🙂

Fair enough, definitely not proposing removing it, especially since we use it 😅

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.

2 participants