Skip to content

platform: refactor: deduplicate code for APL&CNL#121

Merged
lgirdwood merged 2 commits intothesofproject:nextfrom
jajanusz:platforms-refactor
Aug 4, 2018
Merged

platform: refactor: deduplicate code for APL&CNL#121
lgirdwood merged 2 commits intothesofproject:nextfrom
jajanusz:platforms-refactor

Conversation

@jajanusz
Copy link
Copy Markdown
Contributor

@jajanusz jajanusz commented Jul 17, 2018

First step of deduplicating platforms code.
1st commit keeps binaries identical with just removing duplicated code.
2nd commit wraps common code into libtool convinence libs, because we don't want to list common source files for each platform with relative paths.

Next step will be removing #ifdefs and moving small chunks of platform-specific code to their folders, then deduplicating header files.

It is critical for maintainability of platform-specific sources (and adding new platforms in the future).

@jajanusz jajanusz requested a review from lgirdwood July 17, 2018 21:47
@jajanusz jajanusz force-pushed the platforms-refactor branch from a34c028 to 68f7149 Compare July 17, 2018 22:27
Copy link
Copy Markdown
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Missing a commit message. Can you also do this PR against -next as master is in code freeze (bug fixes only).

@jajanusz jajanusz changed the base branch from master to next July 18, 2018 08:34
@jajanusz jajanusz changed the base branch from next to master July 18, 2018 08:34
@jajanusz
Copy link
Copy Markdown
Contributor Author

What do you mean by missing commit message? There are commit messages for each commit and title of PR, do you mean something else?

@lgirdwood
Copy link
Copy Markdown
Member

The commit message is only showing the title and SoB. It should also have a statement saying what is changing and why (especially important for big change sets like this one).

@jiawang6
Copy link
Copy Markdown

CI observed building failure.

Failed Platform: baytrail

make[5]: *** No rule to make target '../../platform/baytrail/libplatform.la', needed by 'sof'. Stop.

@jajanusz
Copy link
Copy Markdown
Contributor Author

jajanusz commented Jul 19, 2018

@jiawang6
Probably you don't have latest commit, looks like your CI ignores amended commit.
I initially did PR that would give error like one that you wrote but within ~5 minutes from initial commit I did commit --amend with fix for that. When is your CI exactly trigerred? Only on pull request? What about updates to PR?

@jiawang6
Copy link
Copy Markdown

@jajanusz , sorry, CI was blocked by Jenkins CI maintaining at that time. I have manually rerun the updated PR and the result is successful.

@lgirdwood
Copy link
Copy Markdown
Member

@jajanusz this is fine, but lets merge into -next since we are in code freeze on master. You may need to update your PR so I can merge into -next.

@jajanusz
Copy link
Copy Markdown
Contributor Author

@lgirdwood I know, just need some changes to make it mergable to next

@jajanusz jajanusz force-pushed the platforms-refactor branch from 68f7149 to 62528f5 Compare August 1, 2018 11:53
@jajanusz jajanusz changed the base branch from master to next August 1, 2018 11:54
@jajanusz jajanusz closed this Aug 1, 2018
@jajanusz jajanusz force-pushed the platforms-refactor branch from 62528f5 to a292d45 Compare August 1, 2018 12:17
First step of deduplicating platforms code.
Keeps binaries identical with just removing duplicated code.

It is critical for maintainability of platform-specific sources (and adding new
platforms in the future).

Signed-off-by: Janusz Jankowski <janusz.jankowski@linux.intel.com>
Wraps common code into libtool convinence libs, because we don't want to list
common source files for each platform with relative paths.

Signed-off-by: Janusz Jankowski <janusz.jankowski@linux.intel.com>
@jajanusz jajanusz reopened this Aug 2, 2018
@jajanusz
Copy link
Copy Markdown
Contributor Author

jajanusz commented Aug 2, 2018

Rebased and updated for next

@lgirdwood lgirdwood merged commit 50c3171 into thesofproject:next Aug 4, 2018
@jajanusz jajanusz deleted the platforms-refactor branch August 9, 2018 09:05
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