Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Container2step #300
Container2step #300
Changes from 6 commits
41b393d
69a490e
7e511b3
5fcc5be
c47471d
a1655ff
9dddc17
cb69264
c5a6de8
709207c
3f788ab
86d4aa3
321e368
245080c
e68f49d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this
try
/except
clause just checking to see if a dictionary's attribute is defined at all? or is it about the boolean value assigned elsewhere?if it's about being defined- how about using
p.get("key")
, then checking forNone
? or catching theKeyError
explicitly?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit of both I think? With some platforms, the
container
key doesn't even need to be defined. So the try/except checks for if it's even there under a certain platform's information (and not just that it's not defined, beingNone
. If it doesn't exist, it'll set things as empty or False for the nextif
statement it goes throughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think maybe this could be done a different way that's slightly easier to follow but my brain keeps going back and forth on logic and is confusing itself. Not sure what's the best way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try just passing
create_makefile_script.makefile_create(yamlfile_path,CONTAINER_PLAT2,TARGET)
instead. I'm pretty sure either theplatformfre.py
oryamlfre.py
file is looking for a list that is passed. I used something likecontainer_plat = CONTAINER_PLAT2[0]
in some other tests mainly for assertions for directory names I think.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That worked!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there any return-codes to check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a copy/paste of the single stage tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these shouldn't be separate- lets do something like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's done here is consistent with the rest of the testing done in this file.