Refactor RuleEngine states #332
Conversation
zigen
force-pushed
the
refactor-rule-engine-states
branch
2 times, most recently
from
cf77594 to
eb698fe
Compare
zigen
force-pushed
the
refactor-rule-engine-states
branch
from
eb698fe to
fb5cb0f
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Naphann and @zigen)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
realtime_controller->ReInitialize_StationaryQubit(qubit_record, false); qubit_record->setBusy(false); if (qubit_record->isAllocated()) {
do we need this check? If in the end we want it to always be false?
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Naphann and @zigen)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
Previously, Naphann (Naphan Benchasattabuse) wrote…
do we need this check? If in the end we want it to always be
false?
I mean, should setBusy care about the current state of the qubit?
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Naphann)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
Previously, Naphann (Naphan Benchasattabuse) wrote…
I mean, should setBusy care about the current state of the qubit?
for now, setBusy cares about the current state of the qubit.
basically, I copied that from the original implementation.
quisp/quisp/modules/RuleEngine.cc
Line 873 in ee8873a
but it might be useless now. let's remove it later.
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Naphann and @zigen)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
Previously, zigen (Kentaro "zigen" Teramoto) wrote…
for now,
setBusycares about the current state of the qubit.
basically, I copied that from the original implementation.
quisp/quisp/modules/RuleEngine.cc
Line 873 in ee8873a
but it might be useless now. let's remove it later.
Normally when a method can throw due to something like this I'd name it try in front. In this case it would betrySetBusy
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Naphann)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
Previously, Naphann (Naphan Benchasattabuse) wrote…
Normally when a method can throw due to something like this I'd name it try in front. In this case it would be
trySetBusy
that's a good idea!
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Naphann and @zigen)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
Previously, zigen (Kentaro "zigen" Teramoto) wrote…
that's a good idea!
I think I'll approve it for and let's change that later
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @Naphann)
quisp/modules/QRSA/RuleEngine/RuleEngine.cc, line 1205 at r1 (raw file):
Previously, Naphann (Naphan Benchasattabuse) wrote…
I think I'll approve it for and let's change that later
okay, let's refactor it later. thanks!
|
@Naphann thanks for reviewing, if it's okay, just LGTM it plz. |
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @Naphann)
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @zigen)
prev: #331
This change is