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

🔥 Feature: add ability to print custom message on startup #2491

Merged
merged 25 commits into from
Jun 19, 2023

Conversation

Saman-Safaei
Copy link
Contributor

@Saman-Safaei Saman-Safaei commented Jun 2, 2023

add a function in listener.go to print out the developer provided custom message

@welcome
Copy link

welcome bot commented Jun 2, 2023

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@efectn
Copy link
Member

efectn commented Jun 2, 2023

I think it's better idea to improve onListen hook instead of adding new config

@Saman-Safaei Saman-Safaei reopened this Jun 2, 2023
@gaby
Copy link
Member

gaby commented Jun 2, 2023

I think it's better idea to improve onListen hook instead of adding new config

Agree

@ghost
Copy link

ghost commented Jun 2, 2023

I think it's better idea to improve onListen hook instead of adding new config

@efectn So, something like this? (this is just a mock-up, and does not take prefork into account)

func (app *App) SetStartupMessage(fun ...func() error) {
	// disable default startup message
	app.config.DisableStartupMessage = true

	// add each func to the onListen hook
	app.hooks.onListen = append(app.hooks.onListen, fun...)
}

and it allows for this:

app := fiber.New()

app.SetStartupMessage(
	func() error {
		log.Println("Hello, ")
		return nil
	}, func() error {
		log.Println("World!")
		return nil
	},
)

log.Fatal(app.Listen(":9090"))

2023/06/02 15:03:13 Hello, 
2023/06/02 15:03:13 World!
^Csignal: interrupt

@Saman-Safaei Saman-Safaei changed the title feat: add ability to print custom message on startup 🔥 Feature: add ability to print custom message on startup Jun 2, 2023
@efectn
Copy link
Member

efectn commented Jun 2, 2023

I think it's better idea to improve onListen hook instead of adding new config

@efectn So, something like this? (this is just a mock-up, and does not take prefork into account)

func (app *App) SetStartupMessage(fun ...func() error) {
	// disable default startup message
	app.config.DisableStartupMessage = true

	// add each func to the onListen hook
	app.hooks.onListen = append(app.hooks.onListen, fun...)
}

and it allows for this:

app := fiber.New()

app.SetStartupMessage(
	func() error {
		log.Println("Hello, ")
		return nil
	}, func() error {
		log.Println("World!")
		return nil
	},
)

log.Fatal(app.Listen(":9090"))

2023/06/02 15:03:13 Hello, 
2023/06/02 15:03:13 World!
^Csignal: interrupt

I think it's better idea to improve onListen hook instead of adding new config

@efectn So, something like this? (this is just a mock-up, and does not take prefork into account)

func (app *App) SetStartupMessage(fun ...func() error) {
	// disable default startup message
	app.config.DisableStartupMessage = true

	// add each func to the onListen hook
	app.hooks.onListen = append(app.hooks.onListen, fun...)
}

and it allows for this:

app := fiber.New()

app.SetStartupMessage(
	func() error {
		log.Println("Hello, ")
		return nil
	}, func() error {
		log.Println("World!")
		return nil
	},
)

log.Fatal(app.Listen(":9090"))

2023/06/02 15:03:13 Hello, 
2023/06/02 15:03:13 World!
^Csignal: interrupt

Hooks handlers accept parameters as you can see here https://github.com/gofiber/fiber/blob/master/hooks.go#L8

We can pass needed variables to hook like this. However, it'll break handler signature. Perhaps we put them into a struct and pass it as variadic, so it won't break compatibility

@efectn
Copy link
Member

efectn commented Jun 9, 2023

I'd recommend something like this:

type ListenData struct {
	Host string
	Port string
	TLS  string
}

app.Hooks().OnListen(func(data ...ListenData) error {
	if fiber.IsChild() {
		return nil
	}

	fmt.Print(data[0].Host)

	return nil
})

Note: Using variadic not to break backward-compatibility.

@Saman-Safaei Saman-Safaei reopened this Jun 11, 2023
app.go Outdated Show resolved Hide resolved
Copy link
Member

@ReneWerner87 ReneWerner87 left a comment

Choose a reason for hiding this comment

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

check my hint

@Saman-Safaei
Copy link
Contributor Author

check my hint

I fixed that

@efectn
Copy link
Member

efectn commented Jun 12, 2023

@Saman-Safaei can you add unit tests and update docs. Also check hooks tests if there are invalid ones

@Saman-Safaei Saman-Safaei requested review from a user and ReneWerner87 June 13, 2023 15:26
@Saman-Safaei Saman-Safaei requested a review from efectn June 13, 2023 15:45
docs/guide/hooks.md Outdated Show resolved Hide resolved
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Can you take a look at the new linter warnings/errors?

listen.go Outdated Show resolved Hide resolved
listen.go Outdated Show resolved Hide resolved
@Saman-Safaei Saman-Safaei requested a review from a user June 13, 2023 20:00
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please revert the f98133f commit.
While the go.mod does specify go 1.20, tests still run with go 1.17-1.20, and 1.17 does not contain any yet.

@efectn
Copy link
Member

efectn commented Jun 14, 2023

@Saman-Safaei can you add nolint flag for a method that isTLS param is used.

You can check other nolint flag usages for an example

@Saman-Safaei Saman-Safaei requested a review from efectn June 14, 2023 22:06
@ReneWerner87 ReneWerner87 merged commit ed95fa8 into gofiber:master Jun 19, 2023
17 checks passed
@welcome
Copy link

welcome bot commented Jun 19, 2023

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

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.

None yet

4 participants