Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

refactor: rename disk_aio to aio_context #311

Merged
merged 8 commits into from
Sep 16, 2019

Conversation

neverchanje
Copy link
Contributor

In many places, disk_aio is aliased to xxx_aio_context. For example:

  • Its subclasses are called posix_disk_aio_context and linux_disk_aio_context
  • disk_engine::prepare_aio_context generates a new disk_aio object.

So make it called aio_context sounds more natural than disk_aio.

foreverneverer
foreverneverer previously approved these changes Sep 16, 2019
@hycdong
Copy link
Contributor

hycdong commented Sep 16, 2019

是不是改成disk_aio_context更好?

@neverchanje
Copy link
Contributor Author

@hycdong 我们的 RPC 没有 aio 的说法,所以 aio 可以待指 disk aio,名字简单点比较容易使用。

char buffer[128];
// in simulator environment this task will be executed immediately,
// so we excluded config-test-sim.ini for this test.
auto t = file::read(fp, buffer, 128, 0, LPC_TASK_TEST, nullptr, nullptr);

t->wait(10000);
ASSERT_TRUE(t->_wait_event.load() != nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

这个为什么去掉?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

会导致单测失败

Copy link
Member

Choose a reason for hiding this comment

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

这个不是本次修改引入的单测失败吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的 @acelyc111 但是晚修不如早修

acelyc111
acelyc111 previously approved these changes Sep 16, 2019
qinzuoyan
qinzuoyan previously approved these changes Sep 16, 2019
@neverchanje neverchanje merged commit 4049f32 into XiaoMi:master Sep 16, 2019
@neverchanje neverchanje deleted the refactor2 branch September 16, 2019 11:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants