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: add SHOW_GITHUB config #371

Merged
merged 1 commit into from
Aug 4, 2024
Merged

feat: add SHOW_GITHUB config #371

merged 1 commit into from
Aug 4, 2024

Conversation

dreamhunter2333
Copy link
Owner

@dreamhunter2333 dreamhunter2333 commented Aug 4, 2024

User description

#368


PR Type

Enhancement, Documentation


Description

  • Added SHOW_GITHUB configuration to control the visibility of the GitHub link in the frontend.
  • Updated the settings API to include the showGithub parameter.
  • Documented the new SHOW_GITHUB configuration in both English and Chinese documentation.
  • Updated the changelog to reflect the addition of the SHOW_GITHUB configuration.

Changes walkthrough 📝

Relevant files
Enhancement
Header.vue
Add conditional visibility for GitHub menu option               

frontend/src/views/Header.vue

  • Added showGithub condition to GitHub menu option visibility.
+1/-0     
commom_api.ts
Include `showGithub` in settings API response                       

worker/src/commom_api.ts

  • Added showGithub to the settings API response.
+1/-0     
types.d.ts
Add `SHOW_GITHUB` to `Bindings` type definition                   

worker/src/types.d.ts

  • Added SHOW_GITHUB to Bindings type definition.
+1/-0     
Documentation
CHANGELOG.md
Document `SHOW_GITHUB` configuration in changelog               

CHANGELOG.md

  • Documented the addition of SHOW_GITHUB configuration.
+1/-0     
cli.md
Add `SHOW_GITHUB` configuration documentation (EN)             

vitepress-docs/docs/en/cli.md

  • Added documentation for SHOW_GITHUB configuration.
+1/-0     
worker.md
Add `SHOW_GITHUB` configuration documentation (ZH)             

vitepress-docs/docs/zh/guide/cli/worker.md

  • Added documentation for SHOW_GITHUB configuration.
+1/-0     
wrangler.toml.template
Add `SHOW_GITHUB` configuration example                                   

worker/wrangler.toml.template

  • Added SHOW_GITHUB configuration example.
+1/-0     

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 2 labels Aug 4, 2024
Copy link

github-actions bot commented Aug 4, 2024

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Key issues to review

Possible Bug
The show property for the GitHub menu option is added after the icon property. This might cause issues if the order of properties is significant in the framework being used. Consider placing the show property before the icon property to ensure consistency.

Copy link

github-actions bot commented Aug 4, 2024

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Possible issue
Move the show property inside the object to ensure it is part of the correct menu item configuration

The show property should be placed inside the object that starts on line 192 to
ensure it is part of the correct menu item configuration.

frontend/src/views/Header.vue [195-196]

-show: openSettings.value?.showGithub,
-key: "github"
+key: "github",
+show: openSettings.value?.showGithub
 
Suggestion importance[1-10]: 7

Why:

7
Add a default value for SHOW_GITHUB to handle undefined environment variables

Consider adding a default value for SHOW_GITHUB to handle cases where the
environment variable might be undefined.

worker/src/commom_api.ts [38]

-"showGithub": getBooleanValue(c.env.SHOW_GITHUB),
+"showGithub": getBooleanValue(c.env.SHOW_GITHUB) || false,
 
Suggestion importance[1-10]: 7

Why:

7
Best practice
Use a consistent type for the SHOW_GITHUB environment variable

Ensure consistency by using the same type for SHOW_GITHUB as other boolean
environment variables.

worker/src/types.d.ts [38]

-SHOW_GITHUB: string | boolean | undefined
+SHOW_GITHUB: boolean | undefined
 
Suggestion importance[1-10]: 7

Why:

7
Enhancement
Provide more context for the SHOW_GITHUB configuration

Add a brief explanation for the SHOW_GITHUB configuration to provide more context
for users.

vitepress-docs/docs/en/cli.md [112]

-# SHOW_GITHUB = true # Show GitHub link
+# SHOW_GITHUB = true # Show GitHub link in the header menu
 
Suggestion importance[1-10]: 7

Why:

7

@dreamhunter2333 dreamhunter2333 merged commit 10da337 into main Aug 4, 2024
1 check passed
@dreamhunter2333 dreamhunter2333 deleted the feature/dev branch August 4, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant