feat: add --head and --tail to logs command
#1629
Conversation
…d tail args for the log command shuttle-hq#1600
|
Hello, is my pr ready to be merged |
There was a problem hiding this comment.
Thanks for working on this @biplab5464! I left a first round of comments, but it is a bit difficult to review some of the files since there are a lot of unrelated changes. It seems like a merge error, where you have overwritten the git history and changed the author to yourself, but I'm not quite sure yet, as I don't see any force pushes. Let us know if you have questions, and we can try to help you figure this out. See comments on the files in question.
cargo-shuttle/src/client.rs
Outdated
| let path = match mode { | ||
| "head" => format!("/projects/{project}/deployments/{deployment_id}/logs?head={len}"), | ||
| "tail" => format!("/projects/{project}/deployments/{deployment_id}/logs?tail={len}"), | ||
| _ => format!("/projects/{project}/deployments/{deployment_id}/logs"), | ||
| }; |
There was a problem hiding this comment.
You could use something like this here instead: https://stackoverflow.com/a/30154791. We should also consider creating an enum for mode and putting it in common, so we can use it everywhere we need it.
|
i was not active for some days, i saw it today, i will work on the changes you mentioned and i haven't did anything to the git history, nothing of my knowledge i was asked to just work on normal log and leave the follow log so i had improvised this, can we talk on discord for helping me a little |
Hey @biplab5464, feel free to reach out in the contributors channel if you want more synchronous communication! Regarding the large diffs in this PR, a colleague pointed out that it may be caused by how windows handles line breaks. I found this article on the subject: https://medium.com/bootdotdev/how-to-get-consistent-line-breaks-in-vs-code-lf-vs-crlf-e1583bf0f0b6, I would try this out on your system to see if it removes the extra changes in the cargo-shuttle args.rs and lib.rs files. |
|
It may be the cause but i do coding in WSL, so i think it shouldn't be a problem but still i will check |
There was a problem hiding this comment.
Thanks for addressing my review @biplab5464! Sorry about the delay in responding, we've had Easter holidays here. And sorry about this PR taking so long, it's turned out to be a bit larger than we expected. But it's getting closer!
I have left some more comments you need to address. There are conflicts with main as well, so I would suggest you first fetch the latest main, then merge it into your branch. Then, address my review and test the changes.
And since this turned out to be more substantial than expected for a good first issue, do let us know if you are not able to continue, or if it has become too time-consuming for you, and we can take it over.
|
yeah it is bit challenging and i like it so much, i am learning a lot of thing. Please guide me as it is. It is a fun learning experience. My laptop charger broke so i couldn't do anything for 2 weeks as it was getting dispatch. So sorry for the delay. Also i fell sick in between. Also i am have job so mostly i able to work on it on weekends mostly. I mean i able to give less time on weekday I will be consistent form now. |
There was a problem hiding this comment.
Did some final fixes, tested and works as expected. Great job @biplab5464
Description of change
Addition of head and taill arguments in the Cargo shuttle log command