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.

Sign up for GitHub

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

Jump to bottom

Get github automation working on ubuntu #903

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

Get github automation working on ubuntu #903

wants to merge 20 commits into from

Conversation

kendonB
Copy link
Collaborator

@kendonB kendonB commented Jun 29, 2021

Get github automation working on ubuntu

Description

A few tweaks to get the automatic checkout pull request feature to work on linux. I think this should also work on Mac.

Related Issue

None.

Motivation and Context

As above.

How Has This Been Tested

This has only been tested on ubuntu.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue or bug)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Renamed existing command phrases (we discourage this without a strong rationale).

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have checked that my code does not duplicate functionality elsewhere in Caster.
  • My code implements all the features I wish to merge in this pull request.
  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests pass.

Maintainer/Reviewer Checklist

  • Basic functionality has been tested and works as claimed.
  • New documentation is clear and complete.
  • Code is clear and readable.

kendonB
kendonB commented Jun 29, 2021
View reviewed changes
castervoice/lib/settings.py
@@ -301,7 +312,7 @@ def _get_defaults():

# EXECUTABLES
"AHK_PATH":
str(Path(_BASE_PATH).joinpath(ahk_path_default)),
str(Path(ahk_path_default)),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one I'm not so sure of. I don't think that this was ever right

Copy link
Member

Choose a reason for hiding this comment

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

Yes as ahk_path_default should already include the base path

@LexiconCode
Copy link
Member

Depending on your time as I know you're busy. A way to improve the code with to be breaking it up into more functions and/or a class. This makes it that only the see the different components and their purpose that makes it easier to test as well.

I'll see if I can get around testing this soon!

@LexiconCode LexiconCode added Enhancement Enhancement of an existing feature Linux OS Issues related to the Linux OS operating system labels Jun 29, 2021
@kendonB
Copy link
Collaborator Author

kendonB commented Jun 30, 2021

Depending on your time as I know you're busy. A way to improve the code with to be breaking it up into more functions and/or a class. This makes it that only the see the different components and their purpose that makes it easier to test as well.

I'll see if I can get around testing this soon!

I don't think I have the time to do a good job of tidying this up just yet. Should I open an issue?

@LexiconCode
Copy link
Member

Depending on your time as I know you're busy. A way to improve the code with to be breaking it up into more functions and/or a class. This makes it that only the see the different components and their purpose that makes it easier to test as well.

I'll see if I can get around testing this soon!

I don't think I have the time to do a good job of tidying this up just yet. Should I open an issue?

Sure

@LexiconCode
Copy link
Member

LexiconCode commented Jul 4, 2021 •
edited
Loading

I still need iron out a few things with Path. You can hold off for a bit on the review comments yet.

@LexiconCode
Copy link
Member

LexiconCode commented Jul 5, 2021 •
edited
Loading

@kendonB

This is unrelated to this PR so keep that in mind. I can't get this to work as it keeps failing despite pulling up MINGW64.

print("Error:" + ahk_script + " neither returned 'activated' nor 'does not exist'")
print("Fallback: load new instance of :" + pattern_match)

I will keep looking into it. I think I can re-implement this work doesn't have to rely on AutoHotkey making it cross-platform using dragonfly. If my understanding is correct github_automation.ahk checks to see if a window title exists or waits for the title to become available. What do you think?

@LexiconCode LexiconCode added the Waiting for User Reply Waiting for OP reply label Jul 5, 2021
LexiconCode and others added 17 commits January 13, 2022 23:25
@kendonB
Copy link
Collaborator Author

kendonB commented Jan 21, 2022

@LexiconCode Yes it would be good if we didn't have to rely on AHK. I think we needed that earlier before dragonfly had that functionality

@LexiconCode LexiconCode added WIP An work in progress and removed Waiting for User Reply Waiting for OP reply labels Jan 22, 2022
@LexiconCode LexiconCode force-pushed the master branch 2 times, most recently from 24ab020 to 88a91bd Compare September 27, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LexiconCode LexiconCode LexiconCode left review comments

Assignees

@kendonB kendonB

Labels
Enhancement Enhancement of an existing feature Linux OS Issues related to the Linux OS operating system WIP An work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kendonB @LexiconCode @dmakarov