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

Check if platform is Linux or Darwin #57

Open
adamtec opened this issue Feb 17, 2021 · 3 comments
Open

Check if platform is Linux or Darwin #57

adamtec opened this issue Feb 17, 2021 · 3 comments

Comments

@adamtec
Copy link

adamtec commented Feb 17, 2021

I know MacOS is not a primary focus but if you could change this if statement to check for both Linux or Darwin this would prevent littering MacOS filesystem with c:/.

Change this:

if thisOS == 'Linux':

to something like:

if thisOS in ('Linux', 'Darwin'):

at following location:

https://github.com/TeledyneLeCroy/SignalIntegrity/blob/652c4d254daa26a7cbede8e4dccdc44bbc92585a/SignalIntegrity/App/Preferences.py#L33

@PetePupalaikis
Copy link
Contributor

Are you saying this is a valid change to make globally? meaning I can replace all such queries with this syntax? Have you made this type of change and tried it? I have never in my life touched a Mac and would have no idea how I would test this. However, if you would agree to run the unit tests on this after making such a change, we can try it.

Pete

@adamtec
Copy link
Author

adamtec commented Apr 2, 2021

Sorry for late reply. I'd say yes that typically can treat Mac and Linux similarly with respect to file paths, etc. There is another open issue that regarding tkinter state argument that prevented this app from being used on MacOS. If you want I can make a fresh pull from your InNextRelease, make minimal changes to work on MacOS, and provide a pull request.

@PetePupalaikis
Copy link
Contributor

Hi there:

Yes, if you make minimal changes and issue a pull request, I will merge it, provided that you run all of the unit tests on you MacOS machine first. The way I would recommend doing this is by examining the files CoverageTest.bat (for windows), CoverageTest.sh (Python 2.7 in Linux) and CoverageTest3.sh (Python 3.X in Linux) and adapting an appropriate MacOS version. These tests are very comprehensive and take 10-15 minutes to complete (showing also code coverage). Alternatively, I run the python script TestAll.py from within my Eclipse IDE. I ask that you do this because I'd have no way of running these tests without creating a MacOS vm, which I don't want to do.

After you run the unit tests and submit, I will rerun on Linux and windows with your changes to confirm they don't break things.

Thanks for contributing!

Pete

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

No branches or pull requests

2 participants