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

Drive app name via Base's "Name" field #1429

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Drive app name via Base's "Name" field #1429

merged 1 commit into from
Oct 30, 2024

Conversation

joepavitt
Copy link
Collaborator

Description

  • Removes the hardcoded name in the manifest.
  • In theory, this should be served up dynamically on each request, but when using dev tools, the name wasn't updated until after a Node-RED restart which is odd, but it does update/work.
  • Updated documentation too

Related Issue(s)

Closes #1375

@joepavitt joepavitt requested a review from Steve-Mcl October 29, 2024 17:19
Copy link
Contributor

@Steve-Mcl Steve-Mcl left a comment

Choose a reason for hiding this comment

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

I was thinking these should have fallback values since a config node (or any node) typically does NOT require a name to be set.

However, this permits users to set the name deliberately empty.

@joepavitt
Copy link
Collaborator Author

Confused by the feedback, are you saying it's okay to proceed as-is?

@Steve-Mcl
Copy link
Contributor

Confused by the feedback, are you saying it's okay to proceed as-is?

😄 Sorry.

Yes, it is approved - I am happy to proceed.

I was musing in the hope you would say "yeah, i deliberately didn't add fallbacks for that reason" OR "hmmm, yeah, we really should have a fallback for these values"

In truth, either is acceptable & just wanted you highlight the pause i had when reviewing - giving you an opportunity to ignore/proceed/adjust 👼

@joepavitt joepavitt merged commit 65a8d82 into main Oct 30, 2024
1 of 2 checks passed
@joepavitt joepavitt deleted the 1375-manifest-name branch October 30, 2024 09:22
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.

Sync the dashboard name in the UI with the base node Name field
2 participants