Skip to content

Conversation

@linglingye001
Copy link
Member

No description provided.

@linglingye001 linglingye001 requested a review from Copilot March 25, 2025 05:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds two new examples that demonstrate how to use Azure App Configuration—one for a console application and one for a Gin web application. Key changes include:

  • A console example (main.go and README) that loads configuration and displays it in a terminal.
  • A Gin web example (main.go and README) that configures routing and HTML rendering based on configuration values.

Reviewed Changes

Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.

File Description
example/console-example/main.go Adds a console app that loads and displays configuration.
example/console-example/README.md Provides usage instructions for the console example.
example/gin-example/main.go Adds a Gin web app that loads configuration and configures routing.
example/gin-example/README.md Provides usage instructions for the Gin example.
Files not reviewed (4)
  • example/console-example/go.mod: Language not supported
  • example/gin-example/go.mod: Language not supported
  • example/gin-example/templates/about.html: Language not supported
  • example/gin-example/templates/index.html: Language not supported

security-scan:
name: Security Scan
runs-on: ubuntu-latest
env:
Copy link
Member

@RichardChen820 RichardChen820 Mar 27, 2025

Choose a reason for hiding this comment

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

Any reason of explicitly setting it to on? According to the doc, it's default value is on

Comment on lines 13 to 27
## Configuration Structure

The example uses a nested configuration structure:

```go
type Config struct {
Font Font
Message string
}

type Font struct {
Color string
Size int
}
```
Copy link
Member

@RichardChen820 RichardChen820 Mar 27, 2025

Choose a reason for hiding this comment

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

This part feels a bit odd, it doesn’t seem necessary to specifically emphasize which struct we’re using here. How about just remove it?

Copy link
Member

@RichardChen820 RichardChen820 Mar 27, 2025

Choose a reason for hiding this comment

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

The rest of this README is focused on explaining how to run the example rather than explaining the example itself. We already explain what's done in this example, no need to emphasize the struct.


// Parse configuration into struct
var config Config
constructOptions := azureappconfiguration.ConstructionOptions{
Copy link
Member

@RichardChen820 RichardChen820 Mar 27, 2025

Choose a reason for hiding this comment

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

The default value is ., how about using the default value instead?

Comment on lines 12 to 26
## Configuration Structure

The example uses a nested configuration structure:

```go
type Config struct {
App App
Message string
}

type App struct {
Name string
DebugMode bool
}
```
Copy link
Member

Choose a reason for hiding this comment

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

Same here, how about to remove this section?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@linglingye001 linglingye001 merged commit 658b699 into release/v0 Mar 28, 2025
15 checks passed
@linglingye001 linglingye001 deleted the linglingye/example branch March 28, 2025 07:55
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.

3 participants