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

par/innovus revert gds.gz output gds extension #880

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

daniellovell
Copy link
Contributor

While I understand this is space-efficient to export the final database as a .gds.gz, this change (introduced in 5e3cfb3 ) significantly breaks the hammer-intech22-plugin fill.mk and icv.mk scripts. See the image below regarding how it breaks the plugin.

This PR reverts the change, so Innovus will export a .gds instead of .gds.gz

The fix could occur on either the hammer-intech22-plugin side, or here. I leave that to the maintainers to decide which is best ;)

Slack_iGwaeFuILT

Related PRs / Issues
N/A

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • Change to core Hammer
  • Change to a Hammer plugin
  • Other

Contributor Checklist:

  • Did you set master as the base branch?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you update the poetry.lock file if you updated the requirements in pyproject.toml?
  • (If applicable) Did you add a unit test demonstrating the PR?
  • (If applicable) Did you run this through the e2e integration tests?
  • (If applicable) Did you update the submodules in e2e/ if this feature depends on updated plugins?

daniellovell and others added 2 commits October 17, 2024 02:08
This change significantly breaks the hammer-intech22-plugin
fill.mk and icv.mk scripts
@vikramjain236 vikramjain236 self-requested a review October 18, 2024 01:37
@vikramjain236
Copy link
Contributor

vikramjain236 commented Oct 18, 2024

Yes, I agree with this. For our last couple of tapeout, I had to do this in my local repo.
We can change it in main hammer. I don't see a major problem with it.

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

Successfully merging this pull request may close these issues.

2 participants