Skip to content

Fix incorrectly linted build() command #709

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

Merged
merged 1 commit into from
Feb 10, 2023

Conversation

vloncar
Copy link
Contributor

@vloncar vloncar commented Feb 10, 2023

Description

Recent reformatting of the code changed the build command in build() of vivado_backend.py. The multi-line string is missing spaces between the parameters. The following error happens:

---------error message---------
Sourcing Tcl script 'build_prj.tcl'
expected boolean value but got "Falsecsim"
    while executing
"if {$opt(reset)} {
    open_project -reset ${project_name}_prj
} else {
    open_project ${project_name}_prj
}"
    (file "build_prj.tcl" line 149)
    invoked from within
"source build_prj.tcl"
    ("uplevel" body line 1)
    invoked from within
"uplevel \#0 [list source $arg] "

Hopefully the linter doesn't complain about this.

Type of change

  • Bug fix (non-breaking change that fixes an issue)

Tests

The Jenkins tests should prove that this works.

Checklist

  • I have installed and run pre-commit on the files I edited or added. -> I actually can't run this and it is Friday evening, so I can't be bothered with fixing my setup.

@vloncar vloncar requested a review from jmduarte February 10, 2023 20:00
@jmduarte jmduarte added the please test Trigger testing by creating local PR branch label Feb 10, 2023
Copy link
Member

@jmduarte jmduarte left a comment

Choose a reason for hiding this comment

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

Tested on my model and it indeed fixes the issues.

@jmduarte jmduarte merged commit 4d326d5 into fastmachinelearning:main Feb 10, 2023
calad0i pushed a commit to calad0i/hls4ml that referenced this pull request Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please test Trigger testing by creating local PR branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants