Bug fix 12 #15

stevenj2019 · 2020-11-07T12:52:46Z

Pull Request,

Bugs discovered in issue have been fixed and tested on 17.x - 19.x mac os Kernels

Ready for push to dev

Bug fix 12

maltfield

Thanks for this PR! A few comments/questions before I import it. Also, did you actually test the build after these changes on all three platforms -- or did you only test the commands?

maltfield · 2020-11-09T11:27:15Z

src/packages/buskill/__init__.py

+
+		if os.file.exists(APP_DIR, 'buskill.conf'):
+			self.CONFIG = dict()
+			config.read('buskill.conf')


shouldn't this be os.path( APP_DIR, 'buskill.conf' )?

Hello Matthew,
apologies I forgot I worked on this while waiting for a VM to come up.

The code is inactive I can amend this and re push code up.

actual feature progress is in work on a different branch on my fork

and yes the line should be if os.file.exists(os.path(APP_DIR, 'buskill.conf'))

sorry, I was referring to the config.read() line -- shouldn't that be config.read( os.path(APP_DIR,'buskill.conf') )?

maltfield · 2020-11-09T11:29:19Z

src/packages/buskill/__init__.py

+		else:
+			do_something = None	
+		# NOTE This a placeholder!!!! 
+		# Here there should be a setup wizard which will create the config file (TODO Make the function to create Config files)


does this suggest that currently the config file is not created if it doesn't yet exist?

This is just a placeholder for now.

On local this has been actually implemented. will be pushed after testing

maltfield · 2020-11-09T11:31:26Z

src/packages/buskill/__init__.py

+		else:
+			msg = "ERROR: Mac Kernel" + self.KERNEL_VERSION + "Unsupported"
+			print( msg ); logger.error(msg)
+# subprocess.run( ['pmset', 'displaysleepnow'] )


wait, when does the pmset command work? Wouldn't it be better to just try a bunch of commands as fall-back instead of giving-up before trying?

the pmset command had the same issue which linked to the bug on my machines on the other version of macOS.

maltfield · 2020-11-09T11:35:14Z

Also, it looks like there still some global var APP_DIR, which should probably be deleted. Everything should use self.APP_DIR since v0.2 switched to a proper BusKilll class, iirc

stevenj2019 · 2020-11-19T09:27:23Z

Also, it looks like there still some global var APP_DIR, which should probably be deleted. Everything should use self.APP_DIR since v0.2 switched to a proper BusKilll class, iirc

I will resolve this within the local code so will be resolved next PR

maltfield · 2020-11-19T21:47:04Z

thanks for the follow-up. Can you please fix the issues above before I merge this PR into the dev branch, instead of after in another PR?

stevenj2019 · 2020-11-30T22:09:43Z

okay, closing PR Will create a new PR

maltfield · 2020-11-30T22:19:31Z

@stevenj2019 I--ummm--why close this PR? You could just push to the branch from which this PR was created, and that would update the existing PR without segmenting our comment history. Or am I missing something?

stevenj2019 · 2020-11-30T22:22:12Z

honestly didn't know it worked like that, I was going to create a fresh PR without my awkward whoopsie in it 😆

maltfield · 2020-11-30T22:28:22Z

I'm not a GitHub expert, but I think that's the process: we commit & comment & test iteratively in the same PR until it's good, and then the PR is accepted/merged.

my awkward whoopsie

I don't think mistaken commits matter if you removed it later in the PR. See the "Files changed" tab -- it's not even present, right?

stevenj2019 · 2020-11-30T22:32:11Z

I pushed the changes to that branch,

we should now see the changed here if that is correct. (I am a bit of a GitHub noob too)

maltfield · 2020-11-30T22:35:15Z

maybe try to unclose this PR?

maltfield · 2020-11-30T22:36:38Z

try another commit & push to stevenj2019/buskill-app in the branch = bug-fix-12

https://github.com/stevenj2019/buskill-app/tree/bug-fix-12

stevenj2019 · 2020-11-30T22:37:31Z

well that worked, and I found more messy comments I forgot about so I will make another commit in a mo

maltfield · 2020-11-30T22:40:07Z

Another thing I want to confirm: are CI builds working for you? Can you go here to setup GitHub Actions on your repo so it actually builds the 3x executables that can be tested before I merge this into the main repo?

https://github.com/stevenj2019/buskill-app/actions

stevenj2019 · 2020-11-30T22:47:39Z

I haven't managed to get them to work, manually nor via CI

as long as it doesn't hit the main branch it should be okay.

In future, if I could make branches on the main repo so I can take advantage of builds and such that would make life much easier, I do not have much time to contribute lately Is all

maltfield · 2020-11-30T22:55:25Z

What do you see when you go to this URL? Does it prompt you to enable GitHub Actions?

https://github.com/stevenj2019/buskill-app/actions

What about here?

https://github.com/stevenj2019/buskill-app/settings/actions

stevenj2019 · 2020-11-30T23:35:14Z

https://github.com/stevenj2019/buskill-app/runs/1476954577?check_suite_focus=true#step:3:674
Build error for macOS

Linux build showed a warning but did not fail

maltfield · 2020-12-01T22:05:42Z

@stevenj2019 Looks like GitHub updated their MacOS shared runners base images, so python3.7 is no longer available (3.8 & 3.9 are).

I updated the build script to be more robust in finding this binary going forward, and now it works:

Can you please pull from the upstream dev branch and merge that into your bug-fix-12 branch and try the build again in your repo?

fixed build script

fix on main repo

This commit makes the MacOS build script more robust so that it's able to survive when GitHub updates their MacOS shared runners python version (and deletes the old python binaires). For more info, see: * BusKill/buskill-app@ec4a023 * https://github.com/buskill/buskill-app/compare/abbb7a4415f0184e4594623d02b6386f856e2fb5..ec4a0239589cfa7b87432cb2e98e0db886abfaa5 * BusKill/buskill-app#15 (comment)

* #15 This commit adds a fall-back command and switches the subprocess function from 'call' to the newer 'run'

stevenj2019 added 3 commits October 14, 2020 13:34

added functionality for catalina

fb67ed0

added command for 17.x

4799b85

fixed for Mojave

20a3f4f

stevenj2019 mentioned this pull request Nov 7, 2020

Delay in locking on MacBook #12

Closed

Merge pull request #2 from stevenj2019/bug-fix-12

64a0bf9

Bug fix 12

maltfield reviewed Nov 9, 2020

View reviewed changes

code cleanup

9374e38

stevenj2019 closed this Nov 30, 2020

removed irrelevant code

d84cdf9

maltfield reopened this Nov 30, 2020

removed more messy code

fd7de2a

changed file metadata

1b40672

stevenj2019 added 2 commits January 19, 2021 19:33

Merge remote-tracking branch 'upstream/dev' into dev

a1bde1a

fixed build script

Merge branch 'dev' into bug-fix-12

f8b74c7

fixed build script

stevenj2019 added 3 commits January 19, 2021 19:40

test commit

fc70bd2

Merge remote-tracking branch 'upstream/dev' into dev

6ba154b

fix on main repo

code cleanup. still no bug fix

1fc111e

stevenj2019 and others added 3 commits January 30, 2021 23:57

code clean, further code testing needed for PR

ad729f5

resolved conflict

7cfc9ae

Update __init__.py

e3f047d

maltfield merged commit 165ec7c into BusKill:dev Sep 26, 2021

maltfield added a commit that referenced this pull request Sep 26, 2021

cleanup of @stevenj2019 PR #15

70b28ac

* #15 This commit adds a fall-back command and switches the subprocess function from 'call' to the newer 'run'

maltfield mentioned this pull request Oct 5, 2021

Cross-Platform USB Archive #22

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug fix 12 #15

Bug fix 12 #15

stevenj2019 commented Nov 7, 2020

maltfield left a comment

maltfield Nov 9, 2020

stevenj2019 Nov 19, 2020

maltfield Nov 19, 2020

maltfield Nov 9, 2020

stevenj2019 Nov 19, 2020

maltfield Nov 9, 2020

stevenj2019 Nov 19, 2020

maltfield commented Nov 9, 2020

stevenj2019 commented Nov 19, 2020

maltfield commented Nov 19, 2020

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020 •

edited

Loading

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020 •

edited

Loading

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020

maltfield commented Nov 30, 2020 •

edited

Loading

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020

stevenj2019 commented Nov 30, 2020

maltfield commented Dec 1, 2020 •

edited

Loading

Bug fix 12 #15

Bug fix 12 #15

Conversation

stevenj2019 commented Nov 7, 2020

maltfield left a comment

Choose a reason for hiding this comment

maltfield Nov 9, 2020

Choose a reason for hiding this comment

stevenj2019 Nov 19, 2020

Choose a reason for hiding this comment

maltfield Nov 19, 2020

Choose a reason for hiding this comment

maltfield Nov 9, 2020

Choose a reason for hiding this comment

stevenj2019 Nov 19, 2020

Choose a reason for hiding this comment

maltfield Nov 9, 2020

Choose a reason for hiding this comment

stevenj2019 Nov 19, 2020

Choose a reason for hiding this comment

maltfield commented Nov 9, 2020

stevenj2019 commented Nov 19, 2020

maltfield commented Nov 19, 2020

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020 • edited Loading

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020 • edited Loading

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020

maltfield commented Nov 30, 2020 • edited Loading

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020

stevenj2019 commented Nov 30, 2020

maltfield commented Nov 30, 2020

stevenj2019 commented Nov 30, 2020

maltfield commented Dec 1, 2020 • edited Loading

maltfield commented Nov 30, 2020 •

edited

Loading

maltfield commented Nov 30, 2020 •

edited

Loading

maltfield commented Nov 30, 2020 •

edited

Loading

maltfield commented Dec 1, 2020 •

edited

Loading