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

ENH: add quick script for repointing afs remotes to github #189

Merged
merged 2 commits into from
Jul 17, 2024

Conversation

ZLLentz
Copy link
Member

@ZLLentz ZLLentz commented Jul 17, 2024

Description

Add afs-remote-fix (afs_remote_fix.py), a tiny script for changing a user's afs remote to point toward their fork and to pcdshub.

Motivation and Context

We're migrating the afs repos so this will be useful.

How Has This Been Tested?

Interactively only

Where Has This Been Documented?

Here only in the readme and in the file

Copy link
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

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

This looks largely good to me

print(f"origin to {new_origin}")
print(f"upstream to {new_upstream}")
confirm = input("Confirm? y/n\n")
if not confirm.lower().startswith("y"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that click is in pcds-conda, click.confirm is a thing https://click.palletsprojects.com/en/8.1.x/prompts/#confirmation-prompts

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, if I switch this to use pcds_conda click is better.

Copy link
Contributor

Choose a reason for hiding this comment

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

I realize I'm just always using pcds-conda so it's not really a time loss for me.

confirm = input("Confirm? y/n\n")
if not confirm.lower().startswith("y"):
return 1
subprocess.run(["git", "remote", "set-url", "origin", new_origin])
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we certain everyone has a remote called origin? Maybe the more consistent thing to do is remove all the remotes then create origin and upstream

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, this is a big assumption. This is definitely what you get if you just git clone from afs. Let me check what happens when you use epics-checkout.

Copy link
Member Author

@ZLLentz ZLLentz Jul 17, 2024

Choose a reason for hiding this comment

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

Here's the wild structure you get from epics-checkout, but it's still origin:

zlentz@psbuild-rhel7-03:~/.../ims/ims-git(master +)$ pwd
/cds/home/z/zlentz/temp/afs_ioc_checks/ims/ims-git
zlentz@psbuild-rhel7-03:~/.../ims/ims-git(master +)$ git remote -v
origin  /afs/slac/g/cd/swe/git/repos/package/epics/ioc/common/ims.git (fetch)
origin  /afs/slac/g/cd/swe/git/repos/package/epics/ioc/common/ims.git (push)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess if someone was savvy enough to change their remote names from the default, they're probably savvy enough to fix this without the help of the script?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's my current thought too, but I'm biased by not wanting to do more work on this. I'm happy to change the behavior if it would be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is good enough for the purpose. probably more important to get this out there for the migration

@ZLLentz
Copy link
Member Author

ZLLentz commented Jul 17, 2024

I'll leave these up for a bit longer to allow for other reviews (from the slack post)

Copy link
Contributor

@slactjohnson slactjohnson left a comment

Choose a reason for hiding this comment

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

LGTM

@ZLLentz ZLLentz merged commit f5e2d87 into pcdshub:master Jul 17, 2024
2 checks passed
@ZLLentz ZLLentz deleted the enh_afs_remote_fix branch July 17, 2024 23:55
@ZLLentz
Copy link
Member Author

ZLLentz commented Jul 17, 2024

If someone comes here later from slack: please still do a post-merge review if you'd like, we can change these scripts as needed.

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