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

added color documentation/FAQ-Section #126

Merged
merged 1 commit into from
Feb 17, 2019
Merged

Conversation

Termuellinator
Copy link
Contributor

as per #120 i've added the colors i've found in /src/color.rs to the readme/FAQ.
Please check if i understood everything right in the code ;)

@meain
Copy link
Member

meain commented Feb 13, 2019

@Termuellinator Thanks a lot for the PR, but I think it would be better to use a placeholder image service to show the color. This will avoid having to keep images in the repo.

Something like

![#b00202](https://placehold.it/15/b00202/000000?text=+) User

will give you

#ffffd8 User

@Termuellinator
Copy link
Contributor Author

@meain that is indeed a more elegant solution, thank you - you never stop learning ^^
i've replaced the images with placeholders now.

@Peltoche
Copy link
Collaborator

Hello @Termuellinator ,

Thanks a lot for this PR, it really great!

Copy link
Collaborator

@Peltoche Peltoche left a comment

Choose a reason for hiding this comment

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

LGTM

@Peltoche Peltoche requested a review from meain February 16, 2019 12:37
@Peltoche Peltoche self-assigned this Feb 16, 2019
Copy link
Collaborator

@Peltoche Peltoche left a comment

Choose a reason for hiding this comment

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

In order to keep the history clean, can you squash all your commit together please? There is a lot of "tests" commit which should be into the master.

@Termuellinator
Copy link
Contributor Author

Termuellinator commented Feb 16, 2019

I've learnt how to squash commits together now, but i think using --force wasn't the best idea to push...and now i can't seem to be able to squash away the merging commit (that was created by clicking on "merge branches" here in github), no matter which tutorial i try :/

i don't assume scrubbing this PR and creating a new branch+PR would be good practice, would it?

@meain
Copy link
Member

meain commented Feb 16, 2019

@Termuellinator Force pushing is the way to go.

Just reset the merging commit using

git reset --hard HEAD~1

Now rebase it with master using.

git pull upstream master --rebase

I'm guessing upstream is the name that you have set for this remote.

Now just force push the rebased commit

@Termuellinator
Copy link
Contributor Author

uhm - now i've got some of your commits in here?! O_o is that a problem or expected behaviour?
but i guess the history is more tidy now ^^

@meain
Copy link
Member

meain commented Feb 16, 2019

@Termuellinator Oh, that went wrong, my bad.

Could you just do a reset using the below command and force push the result.

git reset --hard df1d7ff355d79cd1e70df30259b10a485e710d95

@Termuellinator
Copy link
Contributor Author

Termuellinator commented Feb 16, 2019

that yields
"fatal: Ein 'hard'-Reset mit Pfaden ist nicht möglich."
-> "a 'hard'-reset using paths is not possible"

using just reset without --hard works, but yields no changes when pushing

@meain
Copy link
Member

meain commented Feb 16, 2019

Could you show me the result of

git log --oneline -10

@Termuellinator
Copy link
Contributor Author

415e34e (HEAD -> master, origin/master, origin/HEAD) switch to one line when not outputting to tty
437fbfa Fix the module import paths
5d5511a Drop the support of V1.30.1 and move to the rust 2018 edition
df1d7ff added color documentation/FAQ-Section
e1138bb Refactored classic mode
1b7eec3 Added --classic flag
373a218 Update Cargo.toml
0fa6809 Improve the tree display
73bcd96 Fix some display errors inside the CHANGELOG
5b28831 (cargo-release) start next development iteration 0.12.1-pre

the strange thing is/was that i couldn't see the merger-commit in git log, even after fetch/pull/rebase yet git status says i've got the current master.
Up until the merging git was pretty straight forward, but now i'm more confused about it than i ever was ^^

@meain
Copy link
Member

meain commented Feb 16, 2019

@Termuellinator My bad I had edited the command, just after I posted the message.

@Termuellinator
Copy link
Contributor Author

oh, OK, now i see the error in the reset-command i tried after your first suggestion.
Now there is only one commit, but at the bottom it says "This branch is out-of-date with the base branch" (thats where the merging-commit came from before).
i won't press "update branch" now ^^
but can this PR be merged with this out-of-date message?

@meain
Copy link
Member

meain commented Feb 16, 2019

now do a rebase

@Termuellinator
Copy link
Contributor Author

Termuellinator commented Feb 16, 2019

right, forgot about that one.
But it's saying its up-to-date already:

~/.../git/lsd >>> git reset --hard df1d7ff355d79cd1e70df30259b10a485e710d95                                      ±[●][master]
HEAD ist jetzt bei df1d7ff added color documentation/FAQ-Section
~/.../git/lsd >>> git pull origin master --rebase                                                                ±[●][master]
Von https://github.com/Termuellinator/lsd
 * branch            master     -> FETCH_HEAD
Bereits aktuell.
Aktueller Branch master ist auf dem neuesten Stand.

edit: oh, i got an idea...do i have to add this repository to the git-config file?

@meain
Copy link
Member

meain commented Feb 16, 2019

git pull upstream master --rebase

upstream not origin

@Termuellinator
Copy link
Contributor Author

that yields an error:

~/.../git/lsd >>> git pull upstream master --rebase                                                              ±[●][master]
fatal: 'upstream' does not appear to be a git repository
fatal: Konnte nicht vom Remote-Repository lesen.

Bitte stellen Sie sicher, dass die korrekten Zugriffsberechtigungen bestehen
und das Repository existiert.

i used origin because of your "I'm guessing upstream is the name that you have set for this remote."

now i've learned about remotes and added this one as upstream, now the rebase works and everything seems to finally be in order as far as i can see! :)

thanks so much for your guidance!

one more question for my understanding though: is adding the upstream-repo as a remote a standard procedure that should be done with every project?

@meain
Copy link
Member

meain commented Feb 16, 2019

Yeah, I guess that's a thing people do. It's not like you have to, but you might end up needing it.

Copy link
Member

@meain meain left a comment

Choose a reason for hiding this comment

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

LGTM

@Peltoche
Copy link
Collaborator

Thanks to you guys! Merged

@Peltoche Peltoche merged commit 3d11dd5 into lsd-rs:master Feb 17, 2019
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