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

[#293] Mob timer should work anywhere on the system #295

Conversation

maciej-stanuch
Copy link
Contributor

I've added a check for git availability in getMobTimerRoom method to allow it to work anywhere in the system.

So, now the behaviour is like this:

  • If git repository -> works as it worked before
  • If not a git repository -> use the room name from MOB_TIMER_ROOM

I wonder if the code, especially mob.go file, shouldn't be somehow split into smaller files? It's slightly difficult to work with.
I have not much experience in Go and maybe that's not a good idea to separate all that stuff? I feel like it would make it easier to test it at least.

@maciej-stanuch maciej-stanuch changed the title Mob timer should work anywhere on the system [#293] Mob timer should work anywhere on the system Jul 6, 2022
mob.go Outdated Show resolved Hide resolved
@simonharrer
Copy link
Member

We could even write a test for this. As part of the testbed, there's a directory that is not a git repo. We could switch to that and call the main method with mob timer 0 and verify that we get a specific output in the console. We should change the configuration of the notify command so that no sound output will be generated when running this test.

@simonharrer
Copy link
Member

The issue with long files is discussed at #296

@maciej-stanuch
Copy link
Contributor Author

We could even write a test for this. As part of the testbed, there's a directory that is not a git repo. We could switch to that and call the main method with mob timer 0 and verify that we get a specific output in the console. We should change the configuration of the notify command so that no sound output will be generated when running this test.

Will do, I haven't much free time recently to finish it up properly

@simonharrer simonharrer merged commit 9705b9a into remotemobprogramming:main Jul 12, 2022
@simonharrer
Copy link
Member

Feel free to create a new PR with a test for this.

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