Skip to content

Commit

Permalink
Add class method organization to the Contributing Doc
Browse files Browse the repository at this point in the history
This should help with readability of the project as methods are now
grouped logically.

Also, a couple minor additions were done to the issue template.
  • Loading branch information
jdholtz committed Aug 5, 2023
1 parent d995f98 commit 77691f6
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 17 deletions.
4 changes: 2 additions & 2 deletions .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ body:
attributes:
value: |
Thank you for taking the time to report a bug!
- type: textarea
- type: input
attributes:
label: Version
description: Output of `python southwest.py --version`
Expand Down Expand Up @@ -41,4 +41,4 @@ body:
- type: textarea
attributes:
label: Additional context
description: Add any other context about the problem here such as logs, Google Chrome version, Python version, etc.
description: Add any other context about the problem here such as your Google Chrome version, Python version, etc.
10 changes: 10 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Pull Request (This can be done after submitting the PR or separately by me).
- [Coding Conventions](#coding-conventions)
* [Linting](#linting)
* [Formatting](#formatting)
* [Class Method Organization](#class-method-organization)

## Testing
This project uses [pytest][0] to unit test the application. When adding/modifying the code, you may need to add a new test or modify an existing test.
Expand All @@ -36,6 +37,15 @@ the issue or disable the warning.

It is also highly recommended to use an [EditorConfig][6] plugin for your code editor to maintain a consistent coding style for all project files.

### Class Method Organization
To help with readability, Auto-Southwest Check-In should follow a specific ordering of class methods:
1. Magic methods (such as \_\_init\_\_)
2. Public methods
3. Private methods (prefixed with an underscore)

From there, methods are ordered from top to bottom in the order they are used. Unit tests should be in the same order as the methods they are testing.


[0]: https://docs.pytest.org
[1]: https://pre-commit.com
[2]: https://flake8.pycqa.org/en/latest
Expand Down
4 changes: 2 additions & 2 deletions lib/checkin_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ def _wait_for_check_in(self, checkin_time: datetime) -> None:
# Only try to refresh the headers if the check-in is more than ten minutes away
if sleep_time > 0:
logger.debug("Sleeping until ten minutes before check-in...")
self.safe_sleep(sleep_time)
self._safe_sleep(sleep_time)
self.checkin_scheduler.refresh_headers()

current_time = datetime.utcnow()
sleep_time = (checkin_time - current_time).total_seconds()
logger.debug("Sleeping until check-in: %d seconds...", sleep_time)
time.sleep(sleep_time)

def safe_sleep(self, total_sleep_time: int) -> None:
def _safe_sleep(self, total_sleep_time: int) -> None:
"""
If the total sleep time is too long, an overflow error could occur.
Therefore, the script will continuously sleep in two week periods
Expand Down
6 changes: 3 additions & 3 deletions lib/notification_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ def __init__(self, reservation_monitor: ReservationMonitor) -> None:
self.notification_urls = reservation_monitor.config.notification_urls
self.notification_level = reservation_monitor.config.notification_level

def _get_account_name(self) -> str:
return f"{self.reservation_monitor.first_name} {self.reservation_monitor.last_name}"

def send_notification(self, body: str, level: NotificationLevel = None) -> None:
print(body) # This isn't logged as it contains sensitive information

Expand Down Expand Up @@ -108,3 +105,6 @@ def lower_fare(self, flight: Flight, price_info: str) -> None:
)
logger.debug("Sending lower fare notification...")
self.send_notification(message, NotificationLevel.INFO)

def _get_account_name(self) -> str:
return f"{self.reservation_monitor.first_name} {self.reservation_monitor.last_name}"
2 changes: 1 addition & 1 deletion tests/test_checkin_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def test_safe_sleep_sleeps_in_intervals(
mock_sleep = mocker.patch("time.sleep")

total_sleep_time = weeks * 7 * 24 * 60 * 60
checkin_handler.safe_sleep(total_sleep_time)
checkin_handler._safe_sleep(total_sleep_time)

assert mock_sleep.call_count == expected_sleep_calls

Expand Down
18 changes: 9 additions & 9 deletions tests/test_notification_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,6 @@ def notification_handler(mocker: MockerFixture) -> NotificationHandler:
return NotificationHandler(mock_reservation_monitor)


def test_get_account_name_returns_the_correct_name(
notification_handler: NotificationHandler,
) -> None:
notification_handler.reservation_monitor.first_name = "John"
notification_handler.reservation_monitor.last_name = "Doe"

assert notification_handler._get_account_name() == "John Doe"


def test_send_nofication_does_not_send_notifications_if_level_is_too_low(
mocker: MockerFixture, notification_handler: NotificationHandler
) -> None:
Expand Down Expand Up @@ -120,3 +111,12 @@ def test_lower_fare_sends_lower_fare_notification(

notification_handler.lower_fare(mock_flight, "")
assert mock_send_notification.call_args[0][1] == NotificationLevel.INFO


def test_get_account_name_returns_the_correct_name(
notification_handler: NotificationHandler,
) -> None:
notification_handler.reservation_monitor.first_name = "John"
notification_handler.reservation_monitor.last_name = "Doe"

assert notification_handler._get_account_name() == "John Doe"

0 comments on commit 77691f6

Please sign in to comment.