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

Call it begiak; support case insensitivity; new, working, begiak init script #493

Closed
wants to merge 5 commits into from

Conversation

kj7rrv
Copy link

@kj7rrv kj7rrv commented Dec 2, 2019

No description provided.

Before, .cHoOsE a b did not work.
Now it does.
@mr-martian
Copy link

This should close #488.

@kj7rrv kj7rrv changed the title Call it begiak Call it begiak; support case insensitivity Dec 3, 2019
@kj7rrv kj7rrv mentioned this pull request Dec 3, 2019
@kj7rrv kj7rrv changed the title Call it begiak; support case insensitivity Call it begiak; support case insensitivity; new, working, begiak init script Dec 3, 2019
@jonorthwash
Copy link
Member

You should revert the commits instead of adding new ones that delete the file and readd it.

@kj7rrv
Copy link
Author

kj7rrv commented Dec 4, 2019

Done! This is ready to accept.

@kj7rrv
Copy link
Author

kj7rrv commented Dec 4, 2019

Sorry about all the commits on this. I'm new to git.

@AMR-KELEG
Copy link

Sorry about all the commits on this. I'm new to git.

Hi,

It's a nice opportunity to get introduced to the features of using git.
I would recommend using branches and having multiple pull requests (one for each issue) instead of fixing multiple issues in the same pull request (that's my opinion not sure what the maintainers of the repository think).

You can read more about branches and pull requests here:
https://gist.github.com/vlandham/3b2b79c40bc7353ae95a

@wei2912
Copy link
Member

wei2912 commented Dec 4, 2019 via email

@kj7rrv
Copy link
Author

kj7rrv commented Dec 4, 2019

Okay, can we accept this and I'll do it right in the future? Or will I need to start over?

Copy link
Member

@jonorthwash jonorthwash left a comment

Choose a reason for hiding this comment

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

Please don't remove this module. We're planning to disable it [for now] in the config file, but it doesn't need to be deleted.

@@ -63,6 +64,7 @@ case "$1" in
if [ $? -gt 0 ]; then
exit -1
fi
sleep 1
Copy link
Member

Choose a reason for hiding this comment

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

Is there any guarantee that one second will be long enough for begiak to shut down? Wouldn't it be better to wait for it? I believe there are init scripts that check for a certain period of time (120 seconds??) and then kill it if it doesn't shut down? It would be good to see what those init scripts do.

Copy link
Author

Choose a reason for hiding this comment

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

I think it stops almost instantaneously. It just took longer than the time between two bash commands.

Copy link
Member

Choose a reason for hiding this comment

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

I think it stops almost instantaneously

Under what conditions? The problem is that begiak runs on a low-powered server that hosts other services, and regardless of that you can't predict what exact conditions it might be subject to on any given system.

@kj7rrv
Copy link
Author

kj7rrv commented Dec 5, 2019

Closing for now, will reopen as three different PRs. Sorry about the mess.

@kj7rrv kj7rrv closed this Dec 5, 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.

None yet

5 participants