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

General improvements #6

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

Conversation

dmotte
Copy link

@dmotte dmotte commented Mar 21, 2021

In short, I have made the following changes:

  • improved README and structure
  • added useful --help message to the command
  • improved readability and usability in general
  • removed OS strict check, because this script may also work on other OSes. For example i tested it on Windows with Git Bash and it works

@dmotte
Copy link
Author

dmotte commented Mar 21, 2021

This also resolves #5

@tst2005
Copy link
Owner

tst2005 commented Mar 22, 2021

Hello,
Thanks for the work done.
I will look at your changes.

@tst2005
Copy link
Owner

tst2005 commented Mar 22, 2021

@dmotte on Windows with Git Bash, is the uname command exist ? what is the value return by this uname command ?

@dmotte
Copy link
Author

dmotte commented Mar 22, 2021

@tst2005 this is the output of both uname and uname -a. As you can see, it's not really useful 😅

Motte@dmottepc MINGW64 ~
$ uname
MINGW64_NT-10.0-17763

Motte@dmottepc MINGW64 ~
$ uname -a
MINGW64_NT-10.0-17763 dmottepc 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys

@tst2005
Copy link
Owner

tst2005 commented Mar 22, 2021

@tst2005 this is the output of both uname and uname -a. As you can see, it's not really useful sweat_smile

Motte@dmottepc MINGW64 ~
$ uname
MINGW64_NT-10.0-17763

Motte@dmottepc MINGW64 ~
$ uname -a
MINGW64_NT-10.0-17763 dmottepc 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys

Despite of you tell, It is usefull. I see Windows with Git Bash use the well known mingw-w64 to compile their stuff.
Then I added a MINGW64* support.

@tst2005
Copy link
Owner

tst2005 commented Mar 22, 2021

I will continue soon to update the README about Installation. It need to be improved but i'm not sure to follow what you wrote.

I will keep the verbose option (that you had dropped).
I will keep git-timesync into the bin/directory.
For the rest, I think I apply changes close to your PR (drop --force, add help, etc.).

@dmotte
Copy link
Author

dmotte commented Mar 22, 2021

Thanks for your time. Yeah i know the thing about MINGW64, but i would prefer if there is no check at all and the script assumes to work on all OSes where Bash works... but that's subjective.
Anyway, if you really want to perform that check, i advise you to check for MINGW* instead, so the script will work even on MINGW32 (32-bit) systems.

@tst2005
Copy link
Owner

tst2005 commented Mar 22, 2021

I understand but it is new, I suppose there will be some differences.
After some time, if there is no difference/issue, I will merge them with pleasure!

To be honest I didn't target Bash (even it is most used) but any POSIX shell.
I'm trying to be able to run git-timesync on any unix.

I just realize, I included some part of your work outside of a PR, then it is added like I did it. Sorry for that.

@dmotte
Copy link
Author

dmotte commented Mar 22, 2021

No problem for that. It's your project so feel free to do what you want 😉

@tst2005
Copy link
Owner

tst2005 commented Apr 21, 2022

Hello @dmotte
Your README changes seems nice, I will include them ASAP.
I re-opened your issue about WSL support, I need something to detect properly the WSL case.

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