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.

Sign up for GitHub

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

Jump to bottom

aarch64 crefine: update to match other arches #744

Merged
merged 1 commit into from
Apr 2, 2024
Merged

aarch64 crefine: update to match other arches #744

merged 1 commit into from
Apr 2, 2024

Conversation

corlewis
Copy link
Member

@corlewis corlewis commented Mar 27, 2024 •
edited
Loading

This was a change made in 146eaa1, as part of #696.

@corlewis corlewis self-assigned this Mar 27, 2024
Xaphiosis
Xaphiosis approved these changes Mar 27, 2024
View reviewed changes
Copy link
Member

@Xaphiosis Xaphiosis left a comment

Choose a reason for hiding this comment

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

could use more informative commit message (the "as part of" of your initial comment); optional.

@corlewis
Copy link
Member Author

could use more informative commit message (the "as part of" of your initial comment); optional.

I've updated the commit message but left off the reference to the PR, I didn't think it would be too helpful there. Happy to update again if you disagree and think it would be.

@Xaphiosis
Copy link
Member

could use more informative commit message (the "as part of" of your initial comment); optional.

I've updated the commit message but left off the reference to the PR, I didn't think it would be too helpful there. Happy to update again if you disagree and think it would be.

Cool. I agree with leaving the PR off, but my thought was to add a hint of what this was caused by. I guess a commit reference is good enough if someone's investigating.

@corlewis
Copy link
Member Author

Cool. I agree with leaving the PR off, but my thought was to add a hint of what this was caused by. I guess a commit reference is good enough if someone's investigating.

I could do

This corresponds to a change made in 146eaa1, following adjustments to some no_fail attributes.

That's basically pulling in the commit message of 146eaa1, although I'm not sure if it would be enough to save curious people from needing to look more closely at that commit (and its parent, which makes the lib change).

@lsf37
Copy link
Member

lsf37 commented Mar 27, 2024

Cool. I agree with leaving the PR off, but my thought was to add a hint of what this was caused by. I guess a commit reference is good enough if someone's investigating.

I could do

This corresponds to a change made in 146eaa1, following adjustments to some no_fail attributes.

That's basically pulling in the commit message of 146eaa1, although I'm not sure if it would be enough to save curious people from needing to look more closely at that commit (and its parent, which makes the lib change).

I'd be happy with that, but also happy to follow whatever @Xaphiosis decides.

@corlewis
aarch64 crefine: update to match other arches
1e9ce18 
This corresponds to a change made in 146eaa1, following adjustments to
some no_fail attributes.

Signed-off-by: Corey Lewis <[email protected]>
@corlewis corlewis merged commit a82ad0b into seL4:master Apr 2, 2024
8 of 13 checks passed
@corlewis corlewis deleted the aarch64_crefine_update branch April 2, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xaphiosis Xaphiosis Xaphiosis approved these changes

@lsf37 lsf37 lsf37 approved these changes

@michaelmcinerney michaelmcinerney Awaiting requested review from michaelmcinerney

Assignees

@corlewis corlewis

Labels
cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@corlewis @Xaphiosis @lsf37