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

Removed limitation for --cookie-jar to use only one hurl file #3714

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lilyhuang-github
Copy link

This removes the limitation of only allowing for one hurl file with the --cookie-jar argument and updates the documentation as such.
Should Closes #2537

@lilyhuang-github lilyhuang-github marked this pull request as ready for review February 7, 2025 23:16
Copy link
Collaborator

@jcamiel jcamiel left a comment

Choose a reason for hiding this comment

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

Hi @lilyhuang-github some changes to be address; also could you please squash you commits to a single one and you have your commits to be signed (there is so information on it in the CONTRIBUTING guide).

sign

@lilyhuang-github
Copy link
Author

Hi @lilyhuang-github some changes to be address; also could you please squash you commits to a single one and you have your commits to be signed (there is so information on it in the CONTRIBUTING guide).
sign

Hi, I've implemented the changes, squashed into one commit and it should be verified/signed now. Sorry about accidentally changing the old documentation.

Copy link
Collaborator

@jcamiel jcamiel left a comment

Choose a reason for hiding this comment

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

Hi @lilyhuang-github there are still modifications on files that musn't be changed

@lilyhuang-github
Copy link
Author

Hi @lilyhuang-github there are still modifications on files that musn't be changed

Sorry, I thought I changed it back. It should be correct now.

@lilyhuang-github
Copy link
Author

I accidentally added the (only for one session) twice but it should be correct now.

jcamiel
jcamiel previously approved these changes Feb 10, 2025
@lilyhuang-github
Copy link
Author

I changed the integration test output for the cookie-jar since it changed. If it's ok I can rebase it. If it's not I can undo the commit.

@jcamiel
Copy link
Collaborator

jcamiel commented Feb 12, 2025

Hi @lilyhuang-github it's perfect!

@jcamiel
Copy link
Collaborator

jcamiel commented Feb 12, 2025

Hi @lilyhuang-github you have to change also integration/hurl/tests_ok/help.out.pattern

-c, --cookie-jar <FILE>  Write cookies to FILE after running the session (only for one session)

to

-c, --cookie-jar <FILE>  Write cookies to FILE after running the session

@lilyhuang-github lilyhuang-github force-pushed the multipleCookies branch 2 times, most recently from 9f4a197 to dacd2fc Compare February 12, 2025 17:22
@lilyhuang-github
Copy link
Author

Hi @lilyhuang-github you have to change also integration/hurl/tests_ok/help.out.pattern

-c, --cookie-jar <FILE>  Write cookies to FILE after running the session (only for one session)

to

-c, --cookie-jar <FILE>  Write cookies to FILE after running the session

I've added this change and also changed the README.md's cookie-jar explaination and rebased.

jcamiel
jcamiel previously approved these changes Feb 12, 2025
@jcamiel
Copy link
Collaborator

jcamiel commented Feb 12, 2025

@lilyhuang-github just a small fix:

error: `to_string` applied to a type that implements `Display` in `format!` args
   --> packages/hurl/src/main.rs:314:25
    |
314 |             run.filename.to_string()
    |                         ^^^^^^^^^^^^ help: remove this
    |

@lilyhuang-github
Copy link
Author

@lilyhuang-github just a small fix:

error: `to_string` applied to a type that implements `Display` in `format!` args
   --> packages/hurl/src/main.rs:314:25
    |
314 |             run.filename.to_string()
    |                         ^^^^^^^^^^^^ help: remove this
    |

I've added that fix and rebased now.

@jcamiel
Copy link
Collaborator

jcamiel commented Feb 14, 2025

Hi,
You must run cargo fmt on your code change

Fix formatting

  Changed documentation back and add comments to indicate file

   Revert doc

    Update unit test
@lilyhuang-github
Copy link
Author

Hi, You must run cargo fmt on your code change

Sorry, I've ran the formatting with it now.

@jcamiel
Copy link
Collaborator

jcamiel commented Feb 20, 2025

@lilyhuang-github you've to run cargo fmt at the base of your repo, squash the changes and repush in order to appease the CI/CD

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.

Support --cookie-jar for multiple Hurl files
2 participants