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.

Sign up for GitHub

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

Jump to bottom

Fix-#1640 Added version argument to installation script #1644

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix-#1640 Added version argument to installation script #1644

wants to merge 6 commits into from
+62 −5

Conversation

Tushar-kalsi
Copy link

@Tushar-kalsi Tushar-kalsi commented Mar 4, 2024 •
edited

Related Issue

  • Info about Issue or bug

Closes: #1639

Describe the changes you've made

A clear and concise description of what you have done to successfully close your assigned issue. Any new files? or anything you feel to let us know!

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Please let us know if any test cases are added

No test are added.

Please describe the tests(if any). Provide instructions how its affecting the coverage.

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

A clear and concise description of it.
1: Added version in url from where tar files are downloaded in functions.
2: Added version in docker pull command .
3: Updated documentation and way to install specific version .

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Screenshots (if any)

Original Updated
original screenshot updated screenshot

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 4, 2024 •
edited

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

github-actions[bot]
github-actions bot reviewed Mar 4, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you and congratulations 🎉 for opening your very first pull request in keploy

@Tushar-kalsi
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

slayerjain added a commit that referenced this pull request Mar 4, 2024
@slayerjain
@Tushar-kalsi has signed the CLA from Pull Request #1644
b3499a3
@Tushar-kalsi
Copy link
Author

@slayerjain can you check my PR , i am not able to pass DCO check .

@slayerjain
Copy link
Member

@Tushar-kalsi you should ensure that all the commits in the branch are signed. Please refer to this - https://stackoverflow.com/questions/13043357/git-sign-off-previous-commits

The DCO only cares of the commits in this particular branch. ie https://github.com/keploy/keploy/pull/1644/commits

Tushar-kalsi and others added 4 commits March 5, 2024 10:34
@Tushar-kalsi
Fix-keploy#1640 Added version argument to installation script
917cd95 
Signed-off-by: Tushar kalsi <[email protected]>
@re-Tick @Tushar-kalsi
fix: adds a nil check before stopping the ebpf hooks (keploy#1635)
4f2a43a 
* fix: adds a nil check before stopping the ebpf hooks

* style: formats the changes in loader.go

Signed-off-by: re-Tick <[email protected]>

* style(proxy.go): formats and removes unwanted debug logs in proxy

Signed-off-by: re-Tick <[email protected]>

---------

Signed-off-by: re-Tick <[email protected]>
Signed-off-by: Tushar kalsi <[email protected]>
@Sarthak160 @Tushar-kalsi
refactor: make logs which doesn't effect test result to debug (keploy…
53b32fc 
…#1642)

Signed-off-by: Sarthak160 <[email protected]>
Signed-off-by: Tushar kalsi <[email protected]>
@re-Tick @Tushar-kalsi
fix: updates the exec cmd to start user application (keploy#1643)
369c751 
* fix(launch.go): runs the user cmd in the user logged shell to preserve user env variables

Signed-off-by: re-Tick <[email protected]>

---------

Signed-off-by: re-Tick <[email protected]>
Signed-off-by: Tushar kalsi <[email protected]>
@Tushar-kalsi
Copy link
Author

@slayerjain Done thanks !! DCO check passed .

@gouravkrosx
Copy link
Member

gouravkrosx commented Mar 17, 2024 •
edited

@Tushar-kalsi Please resolve the merge conflicts. We have removed this \$HOME/.keploy-config:/root/.keploy-config mounting from the docker alias during refactoring.

@re-Tick
Copy link
Contributor

re-Tick commented Apr 19, 2024

@shivamsouravjha please review.

@re-Tick re-Tick closed this Apr 19, 2024
@re-Tick re-Tick reopened this Apr 19, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2024
shivamsouravjha
shivamsouravjha reviewed Apr 19, 2024
View reviewed changes
README.md Outdated
Comment on lines 136 to 138
curl -O https://raw.githubusercontent.com/keploy/keploy/main/keploy.sh && source keploy.sh installKeploy -version=v2.0.0-alpha43
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey Tushar, would it be possible to move out installKeploy back to script?

As that approach would be shorted to execute. and won't be function name dependent

@shivamsouravjha
Copy link
Contributor

hey @Tushar-kalsi can you please address the comments and resolve the merge conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@shivamsouravjha shivamsouravjha shivamsouravjha left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[feature]: Addition of version argument to installation script
6 participants
@Tushar-kalsi @slayerjain @gouravkrosx @re-Tick @shivamsouravjha @Sarthak160