Skip to content

Appowner Testing Pull Request

Jump to bottom
dpefour edited this page Mar 23, 2015 · 14 revisions

Important information about Git branch in this document

Although most of the contributions typically target the master branch, the actual branch accepting contributions depends on the Vivado release and needs to be checked on this page.

In this document, the Git branch can be refered as master or <branch> but the exact Git branch name should be used for all the Git commands of this document.

App Owner Steps for Testing a Pull Request

Make sure you are set up as a normal contributor (name/email/proxy config settings)

To prevent corruption of your working area, it is recommended to perform the following steps inside an empty directory. The directory can be deleted once the pull request has been tested.

  1. The app owner must first verify that the pull request includes the digitally signed doc/legal.txt file if the contributor is a new contributor for this app.

If the contributor is contributing for the first time on this app and has not digitally sign doc/legal.txt, then the pull request cannot be merged for legal reasons.

Any new proc for the app should also have a regression test under ./test and, if possible, a detailed documentation under ./doc .

  1. Create a repository by cloning XilinxTclStore into a temporary directory.
On Windows
git clone https://github.com/Xilinx/XilinxTclStore.git
On Linux
git clone https://<USER>@github.com/Xilinx/XilinxTclStore.git
  1. Enter the repository that has been just checked out
cd XilinxTclStore
  1. Rename and add the Git remotes to make the name more concistent with their traditional definitions:
git remote rename origin upstream
git remote add origin https://<USER>@github.com/<USER>/XilinxTclStore.git

Verify the Git remotes:

git remote -v

The remote origin should point to your GitHub repository and the remote upstream should point to Xilinx GitHub repository.

  1. Check out the branch <branch> that needs to be worked on:
git checkout <branch>
  1. Merge the pull request. This step is based on the pull request number that is unique and incremental for each pull request.
git pull upstream refs/pull/<pull_request_number>/head

For example, if the pull request number is 173:

git pull upstream refs/pull/173/head

  1. Verify that there was no merge conflict. Any conflict must be resolve before proceeding.

  2. Now that the code from the pull request has been merged, the next steps are to verify that the app regression test is not broken and that app version and catalog are updated

  • The Vivado linter must be run on the app scripts
  • The app test suite must be run to verify that the code does not break the regression tests
  • The app version must be incremented if the changes must be pushed to the community
  • The app XML needs and other system files such as tclIndex pkgindex.tcl must be updated to reflect the changes (when applicable)
  1. Increment the app version.

If the app version is not incremented, Vivado won't be able to pull the new changes from GitHub.

The app version is coded inside the Tcl file under the app directory that provides the Tcl package:

package provide ::tclapp::<COMPANY>::<APP> <MAJOR_VERSION>.<MINOR_VERSION>

Only the MINOR_VERSION should be typically incremented. The MAJOR_VERSION should be incremented only when the changes inside the app are profund enough to break the backward compatibility.

For example:

package provide ::tclapp::xilinx::designutils 1.4

Refer to [Create the Package Provider] (Adding-a-New-App-to-the-Repository) for additional information regarding the app version.

  1. Update the app XML

The app XML must be updated to reflect the change in the app. The revision history must be added to the app XML as well.

Refer to [Update the App XML File] (Adding-a-New-App-to-the-Repository) for additional information regarding updating the app XML.

  1. Start Vivado from a different directory and point to the temporary directory where the Xilinx repo has been checked out and the pull request merged:
shell% setenv XILINX_TCLAPP_REPO <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore
shell% setenv XILINX_LOCAL_USER_DATA NO
shell% vivado -nojournal -nolog
  1. If the app is not already installed, it can be installed from the command line with the ::tclapp::install command

Use the ::tclapp::install command to install your app. If the app is already installed, the command will un-install the app first before re-installing it:

::tclapp::install <COMPANY>::<APP>
For example:
::tclapp::install xilinx::designutils

Note: the app can also be uninstalled/installed from the GUI.

  1. Verify that all the expected procs are visible and accessible under the <COMPANY>::<APP> namespace

If some procs are missing, it is most likely because they have not been exported inside the Tcl script where they have been defined.

The command line arguments should also be tested through the help system:

<COMPANY>::<APP>::<PROC> -help
For example:
xilinx::designutils::write_template -help

The list of arguments should be verified as well as the description and return value of each procedure.

  1. Open the GUI and check the list of procs displayed under the app. If some of the exported procs are missing, this is most likely due to the release catalog XML that has not been rebuilt.

Refer to [Update the Release XML File for GUI testing] (Adding-a-New-App-to-the-Repository) for additional information regarding updating the release catalog XML.

  1. Run Vivado linter on the app
vivado% lint_files [glob <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore/tclapp/mycompany/myapp/*.tcl]
  1. Source all the scripts under the app just to make sure that they don't contain some errors that would prevent the app from installing
vivado% foreach file [glob <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore/tclapp/mycompany/myapp/*.tcl] { source -notrace $file }
  1. Run the app test suite
vivado% source <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore/tclapp/mycompany/myapp/test/test.tcl
  1. Add the new files to Git and commit the changes
git status
git add <LIST_OF_NEW_AND_UPDATED_FILES>
git commit -m 'Merge pull request 173. Updated designutils version to 1.4'

Note: Only files under the app area must be commited to GitHub.

  1. Verify that all the files have been commited
git status

If some files have been missed, they should be added now and a new git commit should be issued.

  1. Push your changes to your GitHub repository
git push origin <branch>

  1. Go to GitHub and create a pull request for the branch <branch>

  2. If no issue has been discovered in the pull request during the tests, go to GitHub and add a blessing comment to the Gate Keeper. The merge can only be done once the app owner gives his/her blessing. If there was any issue with the code resulting in failure in the linter or regression test(s), the pull request must be rejected with a comment explaining the issue(s). It is then up to the contributor to fix the issue(s) and re-submit a pull request once the issue(s) have been fixed.

  3. Once the pull request has been merged by the gatekeeper, remove the temporary directory where the Xilinx repo has been checked out

Clone this wiki locally