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.

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

Revert "[arm] Refactor installer and build to allow arm builds targeted at grub platforms" #11599

Closed
wants to merge 1 commit into from

Conversation

qiluo-msft
Copy link
Collaborator

@qiluo-msft qiluo-msft commented Aug 1, 2022

Reverts #11341

Why I did it

  1. The original PR has some issues [arm] Refactor installer and build to allow arm builds targeted at grub platforms #11341 (comment)

  2. The %%EXTRA_CMDLINE_LINUX%% is not replaced to the real value, it has impact on the kernel parameter settings.
    See the log sonic-vs.img.gz.log in the latest master build. In the grub.cfg, the %%EXTRA_CMDLINE_LINUX%% is set in the linux command line.

Installing for i386-pc platform.
Installation finished. No error reported.
Switch CPU vendor is: GenuineIntel
Switch CPU cstates are: disabled
EXTRA_CMDLINE_LINUX=%%EXTRA_CMDLINE_LINUX%%
Installed SONiC base image SONiC-OS successfully
ONIE: NOS install successful: file://dev/vdb/onie-installer.bin

@qiluo-msft
Copy link
Collaborator Author

@alexrallen Please help check. You may redo your original PR after this revert.

Copy link
Contributor

@alexrallen alexrallen left a comment

Choose a reason for hiding this comment

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

What is the reasoning of this revert? The PR description fails to justify such a measure.

  1. I don't see how a revert is justified by non-functional code organization issues. While I would have preferred any issues with the file extensions / organization to be reported with the original PR I do not see how this could not be resolved by an additional PR on top of what already exists.

  2. Your second point refers to an issue that I am under the impression that is being resolved by another PR ([Bug][Build] Fix the variable patterns not replaced issue #11572) What additional issues exist that warrant a revert?

@qiluo-msft
Copy link
Collaborator Author

Close this in favor of #11572

@qiluo-msft qiluo-msft closed this Aug 2, 2022
@lguohan lguohan deleted the revert-11341-sonic_arm_grub branch December 20, 2022 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants