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

fix README section on adding new submodules #43

Merged
merged 2 commits into from
Feb 4, 2019
Merged

fix README section on adding new submodules #43

merged 2 commits into from
Feb 4, 2019

Conversation

zhemao
Copy link
Contributor

@zhemao zhemao commented Jan 28, 2019

No description provided.

@zhemao zhemao requested a review from alonamid January 28, 2019 22:31
Copy link
Contributor

@alonamid alonamid left a comment

Choose a reason for hiding this comment

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

LGTM. Tried with these instructions, and it builds

@@ -396,7 +396,16 @@ to the project template.

git submodule add https://git-repository.com/yourproject.git

Then add `yourproject` to the `EXTRA_PACKAGES` variable in the Makefrag.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is still important to keep this right just so that make still tracks the dependencies. I know that SBT handles the build once it is called but I think this is good to keep right?

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, the case that I am trying to avoid is this... you make a change in your new submodule "yourproject" and try to compile without added to the MakeFrag, then make will see that there is no changed sources and the build will continue without taking into account the updated changes. This is if the firrtl file is already generated.

@abejgonzalez
Copy link
Contributor

Other than my comment, I can also confirm that this works! LGTM

@abejgonzalez
Copy link
Contributor

Just to confirm, is this good to merge @zhemao @alonamid ?

@zhemao zhemao merged commit d01e38e into master Feb 4, 2019
@zhemao zhemao deleted the readme-fix branch February 4, 2019 23:03
albert-magyar pushed a commit that referenced this pull request Jul 18, 2019
* stop exceptions on empty conf files

* emit empty verilog file | warn users

* put else's on same line as closing bracket
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.

3 participants