zpp-2022-rust-seastar / seastar-rs

Idiomatic Rust bindings to the Seastar framework
6 stars 3 forks source link

Add RUSTDOCFLAGS to cargo test #11

Closed sylwiaszunejko closed 1 year ago

sylwiaszunejko commented 1 year ago

Changes provided by this PR:

piodul commented 1 year ago

@sylwiaszunejko the PR description and commit message do not explain why this change is needed. It is not really obvious from looking at the diff. Please include the motivation in the PR desc/commit message - what problem does it fix?

piodul commented 1 year ago

I see that you have updated the PR description to include the reason behind the change. Next time, if the PR is ready for review, please leave a comment or re-request review via the GitHub interface - editing the PR description does not trigger any notifications so I was unaware that it is ready.

piodul commented 1 year ago

@sylwiaszunejko Please also add some description to the commit message. The PR description is not preserved in any way in the git history.

It also looks that for some reason your username and email were put into quotes when committing. GitHub does not associate this commit with your account. Your previous commit on the continuous_integration branch looks fine:

[piodul@potwor2 seastar-rs]$ git log kfernandez31/continuous_integration 
commit 7e4a9bf186f00d44afe0783c67039b571fbe869c (kfernandez31/continuous_integration)
Author: “sylwiaszunejko” <“sylwia.szunejko@gmail.com”>
Date:   Sun Jan 15 18:50:57 2023 +0100

    Add RUSTDOCFLAGS to cargo test

commit adfe8e4b0eb3d181f771a425998c9d8865c04fb9
Author: sylwiaszunejko <sylwia.szunejko@gmail.com>
Date:   Fri Dec 30 08:50:29 2022 +0100

    Create rust.yml with format, install Seastar, build and linter steps

Please adjust the username and email in the commit.

sylwiaszunejko commented 1 year ago

I have changed commit message.