adoptium / TKG

TestKitGen (TKG)
Apache License 2.0
18 stars 96 forks source link

Remove unnecessary macros #159

Open llxia opened 3 years ago

llxia commented 3 years ago

Due to historical reasons, we have D, SQ, and Q macros defined in makefile in TKG and used in test repos.

For example: https://github.com/AdoptOpenJDK/TKG/blob/master/compile.mk#L17 https://github.com/AdoptOpenJDK/TKG/blob/master/makeGen.mk#L26 https://github.com/AdoptOpenJDK/TKG/blob/master/makeGen.mk#L28

It will be more clear and simple if these values can be explicitly written in makefile and test files. For example,

I think we should verify and remove the macros from makefiles and test repos if they are unnecessary.

2001asjad commented 3 years ago

Hi @llxia , can you tell more about this issue like what files to look on and how to approach etc. Thank You PS: I am an Outreachy applicant

llxia commented 3 years ago

Thanks @2001asjad . Using D as an example, we need to remove D in TKG repo and replace ${D} with / in other repos (i.e., AdoptOpenJDK/test, OpenJ9, etc)

2001asjad commented 3 years ago

Ok, I'll work on this issue and we can discuss more of this after I make a PR. Thank you @llxia

smlambert commented 3 years ago

@llxia @renfeiw - for this issue, is it preferred to create a separate makefile for the common macros (D, SQ, Q), that can then be used by all test makefiles (through the use of include statements, see https://stackoverflow.com/questions/19307258/how-to-include-a-makefile-into-an-other-makefile).

@2001asjad - It will be useful for you to search through all of the .mk files in this repo as a starting point and see where those D, SQ and Q variables are defined. You will find that they are defined in multiple places. I believe the preference is to define them in 1 place (perhaps a new file called common.mk) and then include them in the files where they are used.

2001asjad commented 3 years ago

@llxia @renfeiw - for this issue, is it preferred to create a separate makefile for the common macros (D, SQ, Q), that can then be used by all test makefiles (through the use of include statements, see https://stackoverflow.com/questions/19307258/how-to-include-a-makefile-into-an-other-makefile).

@2001asjad - It will be useful for you to search through all of the .mk files in this repo as a starting point and see where those D, SQ and Q variables are defined. You will find that they are defined in multiple places. I believe the preference is to define them in 1 place (perhaps a new file called common.mk) and then include that common.mk file in the files where the variables are used.

Ok thank you @smlambert I'll try working on this, if I get stuck I'll ask it then only

renfeiw commented 3 years ago

@llxia @renfeiw - for this issue, is it preferred to create a separate makefile for the common macros (D, SQ, Q), that can then be used by all test makefiles (through the use of include statements, see https://stackoverflow.com/questions/19307258/how-to-include-a-makefile-into-an-other-makefile).

Yes, that's exactly what I thought how we can enhance this.

llxia commented 3 years ago

Yes, I agree. I think it is easier this way.

2001asjad commented 3 years ago

Hello @smlambert , so I came up with a solution, I can make a new makefile where I'll define D, Q, P etc and remove those from rest of the files and use them in those file via importing them. It's like as you mentioned above