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

bpo-41657: Refactor for variable OBJECT_OBJS variable in Makefile #21993

Closed
wants to merge 2 commits into from
Closed

bpo-41657: Refactor for variable OBJECT_OBJS variable in Makefile #21993

wants to merge 2 commits into from

Conversation

ElianMariano
Copy link

@ElianMariano ElianMariano commented Aug 28, 2020

Done a refactor in the variable OBJECT_OBJS inside Makefile.pre.in.

This change was done because a redundancy found in this variable. And it will benefit the next builds if a source file is created inside the Object folder, because this variable will seek this file automatically.

PS: This is my first PR, please give some feedback if something wrong was found in it.

https://bugs.python.org/issue41657

Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

Hi @ElianMariano

Thanks for your interest on cpython and thanks for your first PR!.

IMHO it's more safety have a list of hardcoded files, to have the most
controlled as possible we can when we want to compile. I'm not sure
if this is true or not, it's just my point of view.

In despite of this I don't have the last words, so would be great
listen a core dev.

@ElianMariano
Copy link
Author

ElianMariano commented Aug 29, 2020

Hi @eamanu

Thanks for your feedback!
I didn't realized it was a hard coded variable for some specific reason,
but anyways i thought it would worth a try.
I'll wait for a new reviewer to give a final answer on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants