bao-project / bao-hypervisor

Bao, a Lightweight Static Partitioning Hypervisor
Apache License 2.0
379 stars 128 forks source link

fix(gicd): fix calculation of gic's number of irqs #134

Closed xioliu closed 6 months ago

xioliu commented 8 months ago

According to the specification, if the value of this fieldis N, the maximum SPI INTID is 32(N+1)-1.

xioliu commented 8 months ago

I changed the commit message as your suggestion, please review, thank you.

danielRep commented 8 months ago

I changed the commit message as your suggestion, please review, thank you.

Please squash the two commits @xioliu , you accidentally created an empty commit.

xioliu commented 8 months ago

Hello Daniel,

When I updated the commit message, github created the 2nd commit. I am not sure how I can squash the two commits as I don't see any buttons or icons for this operation. Please give me a clue, thank you. image

danielRep commented 7 months ago

Hello Daniel,

When I updated the commit message, github created the 2nd commit. I am not sure how I can squash the two commits as I don't see any buttons or icons for this operation. Please give me a clue, thank you. image

Sorry for the time to reply. You just need to use git rebase to perform a squash git rebase -i HEAD~2 Follow the information from the command or just take a quick search, you will find N entries explaining how to do it.

xioliu commented 7 months ago

Hello Daniel,

Please review again.

danielRep commented 7 months ago

Hello Daniel,

Please review again.

You squashed the commits but left the previous commit message. Please amend it to fix(gicd): fix calculation of gic's number of irqs

xioliu commented 7 months ago

Right, I forgot that. Done.

danielRep commented 7 months ago

Right, I forgot that. Done.

Sorry for the pedantic requests @xioliu , but you need to split the commit body message to avoid the gitlint checker throwing this error:

B1 Line exceeds max length (99>80): "According to the specification, if the value of this fieldis N, the maximum SPI INTID is 32(N+1)-1."

xioliu commented 7 months ago

Hopefully it can pass now.

danielRep commented 7 months ago

Hopefully it can pass now.

It's finish! Thanks for the contribution @xioliu

xioliu commented 7 months ago

Thank you for your support.

DavidMCerdeira commented 7 months ago

That's it.

Now please squash de two commits. After this, it is ready to merge.

xioliu commented 7 months ago

Err, I screwed it up. I used "git pull" and "git rebase -i Head~2" and mistakenly kept the old commit. Can I revert the previous operation?

xioliu commented 7 months ago

I hope it works now. Sorry for wrong operation.

DavidMCerdeira commented 7 months ago

No worries @xioliu. However, there's one last issue...

I assume you used the Github UI to add my code suggestion, which marked me as co-author. From my understanding, this requires that I should signoff the commit as well. Given that the modification is trivial there's no need for me to be co-author. Please remove the "Co-authored-by: ..." from the commit message.

Sorry for such complexity in such a simple change. We are just trying to make sure everything is done correctly.

xioliu commented 7 months ago

I don't use Github UI. I just clicked the "sign off and commit suggestion" button on this webpage and "Co-authored-by" message was added automatically. I will try to remove as your request.