search

lamikr / rocm_sdk_builder

Other
132 stars 12 forks source link

Readability merge request #26

Closed JassonCordones closed 4 months ago

JassonCordones commented 4 months ago

Here are the fixes again. Sorry if there's any mistake I'm not that good at git

lamikr commented 4 months ago

I can start testing these now. I may manually modify some of the patches as github says that there are some conflicts with the changes I did couple of days ago. What is the purpose of these vscode files, are you opening this sdk as as a vscode project?

JassonCordones commented 4 months ago

Yes, sorry I forgot to add those to the gitignore.

lamikr commented 4 months ago

I tried to apply patches one by one but there were quite many conflicts due to my own changes when I tried to apply them one by one. As you had merged my changes back to your repo, I cloned your version and just copied all 7 files you modified over my ones. This way I got your changes easlity and will test them now while doing a new build.

Attached is a patch I did made a patch containing all of your changes in one. Could you take a look and try to send it as a new merge request for me on top of my latest git commit? In that way it should then merge automatically ok.

If you are new with git, the simplest way is to do these steps like this

git clone github.com:lamikr/rocm_sdk_builder.git unzip rocm_readibility_patch.zip git am 0001-Readability-refactoring.PATCH git commit -s --amend

-s is there to ask git to add your signoff to the end of the message part.

rocm_readibility_patch.zip

lamikr commented 4 months ago

Hi,

Thank you very much for improvements. I think I have now merged most of your changes in. Except that I needed to take my version of func_repolist_apply_patches because your version was missing coupe of lines which were needed to avoid situation that to src_projects/amd-fftw the same patches from patches/rocm-6.1.1/amd-fftw were tried to apply more than once.

amd-fftw project is little special case because the sources are cloned only once, but they are build by 4 times for different parameters and the old logic did not realize that same patches were tried to apply there by each of those 4.

binfo/020_01_amd_fftw_single_precision.binfo binfo/020_02_amd_fftw_double_precision.binfo binfo/020_03_amd_fftw_long_double_precision.binfo* binfo/020_04_amd_fftw_quad_precision.binfo

Can we close this merge request now? If you want to make more changes, you should reset your master branch to have exactly same commits than my master branch. And then when you make changes, you create a branch called "fix_something" and put your changes there. And then do the merge request. That way your do not need to do merge commits to your master branch and it easer merging your changes.

lamikr commented 4 months ago

If it's ok, we could now close this pull request as changes are in?

JassonCordones commented 4 months ago

Yes, sorry I forgot to sync our branches before trying to apply the patch and though I had reversed it before trying to apply it again. I will try to contribute more if I get some spare time in the following weeks. Thanks for the feedback.