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

146: git-webrev: Add support for file list argument #275

Closed
wants to merge 2 commits into from

Conversation

edvbld
Copy link
Member

@edvbld edvbld commented Nov 27, 2019

Hi all,

please review this pull request that add support for an input argument to git-webrev. The argument is a file that itself contains a list of files to include in the webrev. This input file also specifies the order in which the files should appear in the webrev's index.html file.

The main part of the patch is adding support for a list of paths to ReadOnlyRepository.diff.

One part of the bug that is not addressed yet is the support for specifying - as the input file, git-webrev should then read the list of file names (separated by \n) from stdin. I plan to address this in a follow-up patch since I wanted to keep this patch as small as possible.

Thanks,
Erik

Testing

  • Added a new unit test
  • Manual testing of git-webrev with input file
  • make test passes on Linux x64

Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Issue

SKARA-146: git-webrev: Add support for file list argument

Approvers

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Nov 27, 2019

👋 Welcome back ehelin! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 27, 2019

Webrevs

Copy link
Member

@rwestberg rwestberg left a comment

Looks good!

@openjdk openjdk bot removed the rfr label Nov 28, 2019
@openjdk
Copy link

@openjdk openjdk bot commented Nov 28, 2019

@edvbld This change can now be integrated. The commit message will be:

146: git-webrev: Add support for file list argument

Reviewed-by: rwestberg
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

Since the source branch of this PR was last updated there have been 4 commits pushed to the master branch:

  • d665084: Add support for configuring Jira credentials
  • 1f16634: Save references to materialized branches locally
  • 6e2c5bf: Escape potential unintentional markdown formatting when bridging mails
  • f3afc99: Initial version of issue updater

Since there are no conflicts, your changes will automatically be rebased on top of the above commits when integrating. If you prefer to do this manually, please merge master into your branch first.

  • To integrate this PR with the above commit message, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Nov 28, 2019

files = opts['files']
if files != '':
wanted = set(files.split(','))
Copy link
Member

@rwestberg rwestberg Nov 28, 2019

Choose a reason for hiding this comment

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

Suggested change
wanted = set(files.split(','))
wanted = set(files.split(b','))

This fails on python3

@edvbld
Copy link
Member Author

@edvbld edvbld commented Nov 28, 2019

/integrate

@openjdk openjdk bot closed this Nov 28, 2019
@openjdk openjdk bot added the integrated label Nov 28, 2019
@openjdk
Copy link

@openjdk openjdk bot commented Nov 28, 2019

@edvbld The following commits have been pushed to master since your change was applied:

  • d665084: Add support for configuring Jira credentials
  • 1f16634: Save references to materialized branches locally
  • 6e2c5bf: Escape potential unintentional markdown formatting when bridging mails
  • f3afc99: Initial version of issue updater

Your commit was automatically rebased without conflicts.

Pushed as commit f54b027.

@openjdk openjdk bot removed the ready label Nov 28, 2019
@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 28, 2019

Mailing list message from Erik Helin on skara-dev:

Changeset: f54b027
Author: Erik Helin <ehelin at openjdk.org>
Date: 2019-11-28 10:08:55 +0000
URL: https://git.openjdk.java.net/skara/commit/f54b0272

146: git-webrev: Add support for file list argument

Reviewed-by: rwestberg

! cli/src/main/java/org/openjdk/skara/cli/GitWebrev.java
! jcheck/src/test/java/org/openjdk/skara/jcheck/TestRepository.java
! vcs/src/main/java/org/openjdk/skara/vcs/ReadOnlyRepository.java
! vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java
! vcs/src/main/java/org/openjdk/skara/vcs/hg/HgRepository.java
! vcs/src/main/resources/ext.py
! vcs/src/test/java/org/openjdk/skara/vcs/RepositoryTests.java
! webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants