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

bsp: Bugfix: handle over-mounted mountpoint #4691

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

bsp: Bugfix: handle over-mounted mountpoint #4691

wants to merge 7 commits into from

Conversation

hzyitc
Copy link
Member

@hzyitc hzyitc commented Jan 12, 2023

Description

Split from #4660 as @The-going required.

How Has This Been Tested?

  • Build

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@hzyitc hzyitc force-pushed the bsp-bugfix-over-mounted branch from 046bd21 to 06dda19 Compare January 12, 2023 10:03
@The-going
Copy link
Contributor

@hzyitc Has this been tested on a working device?

@hzyitc
Copy link
Member Author

hzyitc commented Jan 12, 2023

@The-going I only test the followings.

  • packages/bsp/common/etc/update-motd.d/30-armbian-sysinfo
  • packages/bsp/common/usr/lib/armbian/armbian-resize-filesystem

@The-going
Copy link
Contributor

I only test the followings.

I'll check the rest myself.

@The-going
Copy link
Contributor

@hzyitc Accept my congratulations! You have a light hand.
You found and fixed some rotten apples. But that's not all.
I found a few more.
I'll have to do another test for debian.
Now I'm checking for jammy, bananapiM64.

This was referenced Jan 20, 2023
Merged
Merged
@igorpecovnik igorpecovnik added the Ready to merge Reviewed, tested and ready for merge label Feb 12, 2023
igorpecovnik
igorpecovnik previously approved these changes Feb 12, 2023
View reviewed changes
Copy link
Member

@igorpecovnik igorpecovnik left a comment

Choose a reason for hiding this comment

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

we will do testings after rc is out, so better me merge now

@The-going
Copy link
Contributor

merge now

These are system files and they still need to be thoroughly checked while running on the hardware. One pull request for each file will make it easier to do this and will allow you to cancel in case of a critical error.
I wouldn't do a merge this PR.

@igorpecovnik igorpecovnik changed the base branch from master to main February 21, 2023 18:37
@igorpecovnik igorpecovnik dismissed their stale review February 21, 2023 18:37

The base branch was changed.

@igorpecovnik igorpecovnik requested a review from a team as a code owner February 21, 2023 18:37
igorpecovnik
igorpecovnik approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@igorpecovnik igorpecovnik left a comment

Choose a reason for hiding this comment

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

When @The-going approves.

@The-going
Copy link
Contributor

When @The-going approves.

I am currently busy with other work.
I will be able to check and thoroughly test only at a later time. Please ask someone else.

@rpardini
Copy link
Member

Hmm, even though a-n is now main, this is mostly on board-side (not build-time) changes, so should be rebaseable. Let me try...

@rpardini
Copy link
Member

/rebase

@igorpecovnik igorpecovnik added 23.08 Work in progress Unfinished / work in progress and removed Ready to merge Reviewed, tested and ready for merge 23.05 labels May 19, 2023
@igorpecovnik igorpecovnik added Backlog Stalled work that needs to be completed Needs review Seeking for review and removed 23.08 Work in progress Unfinished / work in progress labels Nov 15, 2023
@github-actions github-actions bot added the size/small PR with less then 50 lines label Dec 11, 2023
@ColorfulRhino
Copy link
Collaborator

🧹 PR Cleanup Quest 🧹

Ping @hzyitc @The-going

@The-going
Copy link
Contributor

🧹 PR Cleanup Quest 🧹

Just close it.

@ColorfulRhino
Copy link
Collaborator

Just close it.

I wanted to give the person some time to respond, especially since this PR was already approved for merging.

If you deem this bugfix as not important, we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igorpecovnik igorpecovnik igorpecovnik approved these changes

@The-going The-going Awaiting requested review from The-going

Assignees
No one assigned
Labels
Backlog Stalled work that needs to be completed Needs review Seeking for review size/small PR with less then 50 lines
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hzyitc @The-going @rpardini @ColorfulRhino @igorpecovnik