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 Feature rename #12 #17

Merged
merged 7 commits into from
Feb 18, 2024
Merged

Add Feature rename #12 #17

merged 7 commits into from
Feb 18, 2024

Conversation

SummerGram
Copy link
Contributor

This is for the rename feature!

@talis-fb talis-fb linked an issue Feb 17, 2024 that may be closed by this pull request
@talis-fb talis-fb self-requested a review February 17, 2024 23:45
Copy link
Owner

@talis-fb talis-fb left a comment

Choose a reason for hiding this comment

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

The general solution is really great! Follow well all the architecture of project. There are just some adjustments

src/view/commands/rename_request.rs Outdated Show resolved Hide resolved
src/view/commands/rename_request.rs Outdated Show resolved Hide resolved
@talis-fb
Copy link
Owner

@SummerGram did you run cargo test? The error of pipeline coming from basic integration tests of code. You can fix it just fixing the MockAppBackend in tests/mocks/repositories/mod.rs. It's really easy to fix, just call the default method following exactly as all the other methods in this implementation.

@SummerGram
Copy link
Contributor Author

btw, I found that there are some typo in the code. Do I require to create new issue and have a PR for that?

@SummerGram
Copy link
Contributor Author

@talis-fb Please have a look!

@SummerGram SummerGram requested a review from talis-fb February 18, 2024 03:52
@talis-fb
Copy link
Owner

btw, I found that there are some typo in the code. Do I require to create new issue and have a PR for that?

You can fix them in this PR! No problemn

@talis-fb
Copy link
Owner

talis-fb commented Feb 18, 2024

@SummerGram are you going to fix the Typo?? If yes, please add a e2e test for your modification too. In tests/e2e/basic.rs. This is the last thing that would be nice to add. If not I can add later. But I've already did the test to add there. You can just paste it.

#[test]
fn should_rename_requests() {
    // Setup
    let input = format!("treq GET {}/get --save-as request-to-be-renamed", host());
    let mut cmd = run_cmd(&input);
    cmd.assert().success();

    // Before rename
    let input = "treq run request-to-be-renamed";
    let mut cmd = run_cmd(&input);
    cmd.assert().success();
    let input = "treq run request-renamed";
    let mut cmd = run_cmd(&input);
    cmd.assert().failure();

    let input = "treq rename request-to-be-renamed request-renamed --no-confirm";
    let mut cmd = run_cmd(&input);
    cmd.assert().success();

    // After rename
    let input = "treq run request-to-be-renamed";
    let mut cmd = run_cmd(&input);
    cmd.assert().failure();
    let input = "treq run request-renamed";
    let mut cmd = run_cmd(&input);
    cmd.assert().success();
}

@talis-fb
Copy link
Owner

If you like you can add some other tests case. It would be awesome. But this one is the sufficient

@SummerGram
Copy link
Contributor Author

@talis-fb

When will it be merged?

@talis-fb
Copy link
Owner

@talis-fb

When will it be merged?

@SummerGram, now. I was just waiting you confirm if you'd up the typo and test.

Really great job! Very thanks for your contribution! 🤩

@SummerGram
Copy link
Contributor Author

@talis-fb

I fix the typo and add the test.

Btw, what is the roadmap and the goal of this repo?

@talis-fb talis-fb merged commit e3407a5 into talis-fb:develop Feb 18, 2024
1 check passed
@talis-fb
Copy link
Owner

@SummerGram

Good question! I really need to put this in somewhere in the repo, I'm working on it.

Basically the repo trying to be a improvement of HTTPie (and all Cli HTTP Clients) with focus on persistent requests and ways to manipulate them easier in terminal. A tool to bring the way we work with GUI app like Insomnia to terminal. Instead just do a "quick requests" (like the workflow of httpie and curl like apps) you have a "collection" of requests with saved body and params, and them you can submit them just changing a couple of field with a little command.
I'd say the goal of TReq is be a terminal tool to make the users do not feel the needing to use GUI app when testing APIs, due to some missing features CLI tools do not provide.

For now, the priority is really implement all the "must have" features. The foundation of HTTPie interface. So, for now, we really does not have any differential except the saved requests. But in the future the ideia is focus in

  • Add a TUI view, like this project: https://github.com/talis-fb/legacy_TReq. With focus in view, editing and submit the saved requests. Being easy to organize and view the saved requests.
  • Add collections of requests. Organizing requests in "folder".
  • Add Env variables in payload and request, like insomnia, with { TOKEN } inside a body or header for example.
  • Add new operators in body declaration of request item (key=value) to use a editor for insert easier values.

@SummerGram
Copy link
Contributor Author

@talis-fb

honestly, I only use CLI app like curl and wget. For the GUI, I used postman before.

Do you try to outperform them? Not sure if it can outperform curl.

@talis-fb
Copy link
Owner

@SummerGram What do you mean with "outperform"??

@SummerGram
Copy link
Contributor Author

@talis-fb

I mean the performance of treq commands is better than other CLI app similar commands

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.

Command for rename saved requests
2 participants