Skip to content

Strip whitespace from revision#3

Merged
b-deam merged 2 commits intoelastic:mainfrom
b-deam:strip-whitespace-revision
Aug 29, 2023
Merged

Strip whitespace from revision#3
b-deam merged 2 commits intoelastic:mainfrom
b-deam:strip-whitespace-revision

Conversation

@b-deam
Copy link
Copy Markdown
Member

@b-deam b-deam commented Aug 29, 2023

You can test this PR yourself (prior to elastic/rally#1772 being merged) by applying this patch to your local rally-tracks checkout:

diff --git a/pyproject.toml b/pyproject.toml
index fae550f..c1f6ba9 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -33,7 +33,8 @@ exclude = [
 [tool.hatch.envs.default]
 dependencies = [
     "esrally[develop] @ git+https://github.com/elastic/rally.git@master",
-    "pytest-rally @ git+https://github.com/elastic/pytest-rally.git@main",
+    "pytest-rally @ file:///home/esbench/pytest-rally",
+    #"pytest-rally @ git+https://github.com/elastic/pytest-rally.git@main",
 ]
 
 [tool.hatch.envs.unit]

And using this invocation:

$ /home/esbench/rally-tracks
$ pip cache remove pytest_rally && hatch env prune  && hatch -v -e it run test

Relates elastic/rally#1772

@b-deam b-deam added the bug Something isn't working label Aug 29, 2023
@b-deam b-deam requested a review from pquentin August 29, 2023 03:19
@b-deam b-deam self-assigned this Aug 29, 2023
Copy link
Copy Markdown
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! I've left optional suggestions.

return run_command_with_output(f'git -C {repo} rev-parse HEAD').strip()
else:
return current.split()[1]
return current.split()[1].strip()
Copy link
Copy Markdown
Member

@pquentin pquentin Aug 29, 2023

Choose a reason for hiding this comment

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

This strip() is not needed as branches was split over "\n" already. But I'm happy to be over-resilient here.

Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>
@b-deam b-deam merged commit 5bc8856 into elastic:main Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants