Remove git clean invocation from DRAM model generation #580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey, any word on this? |
|
Hi David, We are reviewing this change internally and will also kick off some regressions today to make sure the change does not break anything and should be able to get back to you in the next few days. Thank you so much for the follow up Thanks |
|
Just a friendly bump on this. |
|
Hi David, the changes by themselves look good; but we see some timing failures in test_dcp_recipes and other seemingly unrelated failures in Jenkins with this PR. so re-running the jenkins without your changes with a different PR to confirm if its the jenkins setup vs the PR. should know this shortly in the next day. Thanks |
|
Wonderful, thanks! |
AWSjoeluc
approved these changes
Oct 14, 2022
kyyalama2
approved these changes
Oct 14, 2022
AWScsaralay
approved these changes
Oct 14, 2022
|
Hey @kyyalama2, could this get into the next release (REL_2022_1) ? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue #, if available: N/A, though you can find related discussion in firesim/aws-fpga-firesim#59
Description of changes:
Calling
git cleanto purge the DRAM model build directory is an easy and natural thing to do, but it makes it hard to relocate copies ofaws-fpgato remote build instances in cases whenaws-fpgamay be a git submodule of a super project (sinceaws-fpga's.git/will refer to the super project's.git/under modern versions). This is the case in our project (https://github.com/firesim/firesim).I was hoping that instead of doing some ourselves, if this could be changed here. I think all that's necessary is to remove
tmp/to cover cases where the build failed previously, but i added all the files the existinggit cleanwould remove for consistency. Let me know if this change would be welcome, and if so, how'd you like it to be modified / further tested.