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

add branch command (draft) #272

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft

Conversation

milahu
Copy link

@milahu milahu commented May 12, 2022

fix #271

mostly copy-paste of the pr command

  • parser for branch urls
  • function review.build_branch(branch)
  • tests
  • maybe refactor with the pr command

@Mic92
Copy link
Owner

Mic92 commented May 13, 2022

Implementation looks good so far. As you already put it out a test would be nice and also documentation in the README.

@milahu
Copy link
Author

milahu commented May 13, 2022

6fe1fee fixes my caller error

nix-review rev -r https://github.com/delroth/nixpkgs -b gstreamermm-build-fix a9f5b7dbfe16c81a026946f2c9931479be31171d

because the branch gstreamermm-build-fix is a9f5b7dbfe16c81a026946f2c9931479be31171d

also there is a bug in rev. when i call

nix-review rev -r https://github.com/delroth/nixpkgs a9f5b7dbfe16c81a026946f2c9931479be31171d

then nix-review fetches the master branch of https://github.com/delroth/nixpkgs but the fork is out-of-sync with NixOS/nixpkgs, so as base commit i get delroth/nixpkgs@045a4a4 (2018-10-13)

nix-review should get the ahead status of the branch, to find the base commit delroth/nixpkgs@19e187f (2022-05-05)
see stackoverflow: git ahead behind info

@milahu
Copy link
Author

milahu commented Jun 18, 2022

now this works

nixpkgs-review branch https://github.com/milahu/nixpkgs/tree/qt6-631 -C ~/nixpkgs

this PR still needs some polish ...


cache_size_limit = 200 * 1000 * 1000, # 200MB (default: 1GB)

cache = diskcache.Cache(cache_dir, size_limit = cache_size_limit)
Copy link
Owner

@Mic92 Mic92 Jun 19, 2022

Choose a reason for hiding this comment

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

I would prefer to have no dependencies outside the python stdlib in nixpkgs-review. There is for example: https://docs.python.org/3/library/dbm.html and one could just keep the last 100 evals...

Copy link
Author

@milahu milahu Jun 21, 2022

Choose a reason for hiding this comment

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

eval cache is now a "stupid file cache" with *.xml.gz files (todo: garbage collect)

i think it's useful and stable enough to make eval-cache default on

nixpkgs_review/review.py Outdated Show resolved Hide resolved
nixpkgs_review/review.py Outdated Show resolved Hide resolved
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.

wip does not build child packages
2 participants