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

Diff on target change #20

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

diogomatsubara
Copy link

Hi, I wonder if you are interested in this change.

It shows an uniffied diff of the changes before each of the save commands. I found myself moving a lot between targets and never know if I should discard a change or save it. This helps me with that.

It's my first contribution to a go project, so please let me know how to get this into shape to merge into master.

Thank you!

@diogomatsubara
Copy link
Author

I don't understand why the build on Travis failed. Could you help with that?

@guidowb
Copy link
Owner

guidowb commented Aug 14, 2019

Hey @diogomatsubara. Thanks for the PR and apologies for the delay. I'm just now ramping up to take another look at this project.

The Travis failure was most likely caused by a change in the location of one of the dependencies. I saw the same failure on master locally when running go test. I just committed some changes to switch to go modules for dependency management, and to update the location of some of the test dependencies. Travis passes on master now.

I agree with your problem statement and like the suggestion of showing the differences. But I'm not completely in love with the readability of the diff as implemented in your PR. For instance, a simple cf target -s to switch to a different space results in the following output:

✗ cf set-target pws-prod
Your current target has not been saved. Use save-target first, or use -f to discard your changes.
--- Current
+++ Target
@@ -35,6 +35,6 @@
  "SpaceFields": {
   "AllowSSH": true,
-  "GUID": "06d27aeb-7fde-4205-bb70-e883034f3cbf",
-  "Name": "development"
+  "GUID": "3c0a90dd-02a1-46b7-aacf-57f5d53122ac",
+  "Name": "production"
  },
  "Target": "https://api.run.pivotal.io",

I'd love to give some thought to improving the readability.

More importantly, I would like to see code changes accompanied by tests that demonstrate and validate the new functionality. If you're still interested in this project, could you take a look at cf_targets_test.go and add tests that exercise your new code?

@sahil-bawa
Copy link

Over a year of this PR being open. Dagnabbit.

norman-abramovitz added a commit to norman-abramovitz/cf-targets-plugin that referenced this pull request Aug 23, 2024
 from diogomatsubrata on
[PR-20](guidowb#20)
2. I changed his code to redact tokens and secrets.  A sha256
   sum is used to show there are differences without exposing
   the actual values.
3. Changed the unified diff code to not be dependent on an archived
   read-only directoy.
norman-abramovitz added a commit to norman-abramovitz/cf-targets-plugin that referenced this pull request Aug 28, 2024
… displaying a unified diff. (#4)

* Using "github.com/go-corelibs/diff" diff package

* private package for unified diff copied from
https://github.com/golang/tools/tree/v0.24.0

* 1. Show an unified diff of what changed based upon the work
 from diogomatsubrata on
[PR-20](guidowb#20)
2. I changed his code to redact tokens and secrets.  A sha256
   sum is used to show there are differences without exposing
   the actual values.
3. Changed the unified diff code to not be dependent on an archived
   read-only directoy.

* Added new dependencies

* go fmt cleanup

* I could not figure out how to make a local module/package withou
using a replace line.

* 1. resolve <nil> printing after the diff
2. Make the code more idomatic go

* Shorten up the required copyright notice

* Only call unified diff printing if there are changes
norman-abramovitz added a commit to norman-abramovitz/cf-targets-plugin that referenced this pull request Aug 30, 2024
* Using "github.com/go-corelibs/diff" diff package

* private package for unified diff copied from
https://github.com/golang/tools/tree/v0.24.0

* 1. Show an unified diff of what changed based upon the work
 from diogomatsubrata on
[PR-20](guidowb#20)
2. I changed his code to redact tokens and secrets.  A sha256
   sum is used to show there are differences without exposing
   the actual values.
3. Changed the unified diff code to not be dependent on an archived
   read-only directoy.

* Added new dependencies

* go fmt cleanup

* I could not figure out how to make a local module/package without
using a replace line.

* 1. resolve <nil> printing after the diff
2. Make the code more idomatic go

* Shorten up the required copyright notice

* Only call unified diff printing if there are changes

* Added instal target for development building

* 1. A bad version value does not cause a panic
2. Some docs to refresh the brain cells in the future

* Bump version_tag (patch+1) to make Dennis happy

* Fix error message to display the number of version parts

* 1. handle case if there are no tags
2. make clean up less dangerous if we get nasty input

* add some vscode ignore files
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.

3 participants