Skip to content

Add upstream & upstreamfr commands#118

Open
luzzardik wants to merge 3 commits intoAventiumSoftworks:masterfrom
luzzardik:upstreamcmd
Open

Add upstream & upstreamfr commands#118
luzzardik wants to merge 3 commits intoAventiumSoftworks:masterfrom
luzzardik:upstreamcmd

Conversation

@luzzardik
Copy link

Hi,

This PR adds the /upstream and /upstreamfr commands which answers with a guide on how to update a Helios Launcher fork from the original repository.

Copy link
Member

@GeekCornerGH GeekCornerGH left a comment

Choose a reason for hiding this comment

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

A few things here:

  • Not everybody is using ssh with git
  • I'd prefer if you could name the commands update or something like this instead
  • I don't think you need to checkout master. The merge command should be enough
  • Please sign your commits as well

@luzzardik
Copy link
Author

luzzardik commented Jan 30, 2026

  • Not everybody is using ssh with git

I'll use the https link for the original repo, but I doubt Github requires authentication for SSH based pulls on public repositories.

  • I'd prefer if you could name the commands update or something like this instead

Yup, good idea, will change, will call them fetchchanges and fetchchangesfr.

  • I don't think you need to checkout master. The merge command should be enough

In my opinion, we should leave this command to force users to pull from their fork. Preventing them from even more merge conflicts than necessary that could be a mess to handle for a beginner with git.

  • Please sign your commits as well

They're signed, I need to verify my new e-mail on Github, but as any good tech enthusiast out there, I play too much with my personal domain DNS configuration. 😂 Will fix ASAP.

Edit: my bad, they're not. Will sign them.

@GeekCornerGH
Copy link
Member

I'll use the https link for the original repo, but I doubt Github requires authentication for SSH based pulls on public repositories.

They actually do require an SSH key to be linked to a GitHub account

In my opinion, we should leave this command to force users to pull from their fork. Preventing them from even more merge conflicts than necessary that could be a mess to handle for a beginner with git.

I've seen a lot of people cloning the repo then pushing to a non-fork repo, thus rewriting the branch name. Imho it would cause more confusion than anything else. I can't see how it would cause less merge conflicts tho.

Signed-off-by: Kacy Luzzardi <him@luzzardik.net>
@luzzardik
Copy link
Author

  • Not everybody is using ssh with git

Using HTTPS rather than SSH now.

  • I'd prefer if you could name the commands update or something like this instead

Renamed !

  • I don't think you need to checkout master. The merge command should be enough

Read some docs, yup, useless.

  • Please sign your commits as well

Signed all my commits, still shown as "Unverified" on GitHub due to Vigilant mode (still need to verify my email address, will do so ASAP).

"fields": [
{
"title": "1.",
"description": "First, you'll need to add the original repo as a remote origin. If you already done so, you can skip this step.\n`git remote add upstream https://github.com/dscalzi/HeliosLauncher.git`"
Copy link
Member

Choose a reason for hiding this comment

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

You should mention they need to skip this step if Git says there's already an origin named Upstream

Copy link
Author

Choose a reason for hiding this comment

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

Isn't "If you already done so, you can skip this step." enough ? ^^'

Copy link
Member

Choose a reason for hiding this comment

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

Probably, but you should mention git might setup upstream by default, mainly if the users are using GitHub Desktop

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.

2 participants