Skip to content

add hide grades feature with --hide_grades flag - #155

Open
ericshermancs wants to merge 7 commits into
Grade-Notifier:masterfrom
ericshermancs:hide_grades
Open

add hide grades feature with --hide_grades flag#155
ericshermancs wants to merge 7 commits into
Grade-Notifier:masterfrom
ericshermancs:hide_grades

Conversation

@ericshermancs

Copy link
Copy Markdown
Collaborator

No description provided.

number = input(
"Enter phone number: ") if not args.phone else args.phone
school = input(
"Enter college code: ").upper() if not args.school else args.school.upper()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (87 > 79 characters)

Comment thread src/core/grade_notifier.py Outdated
refresh(remaining_attempts - 1)

def start_notifier(is_welcome=False):
def start_notifier(is_welcome=False, hide_grades=False):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Comment thread src/core/grade_notifier.py Outdated
return False

def create_instance():
def create_instance(hide_grades=False):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

.newline() \
.add("You're all set up. You should see your current grades below!") \
.add("You're all set up. You should see your current grades below!"\
if not hide_grades else "") \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

.add("Your UID is: {0}".format(custom_hash(user.get_username()))) \
.newline() \
.add("You're all set up. You should see your current grades below!") \
.add("You're all set up. You should see your current grades below!"\

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the backslash is redundant between brackets

Comment thread src/core/grade_notifier.py Outdated
###********* Main Program *********###

def welcome_message():
def welcome_message(hide_grades=False):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Comment thread src/core/grade_notifier.py Outdated
Changelog: The list of classes which have had grade changes
'''
def create_text_message(change_log, is_welcome=False):
def create_text_message(change_log, is_welcome=False, hide_grades=False):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 0

@CUNY-Bot

CUNY-Bot commented May 28, 2019

Copy link
Copy Markdown
Collaborator
2 Errors
🚫 Please provide a summary in the Pull Request description
🚫 Oh No! You failed a unit test
Run Python3 ./src/tests/tests.py to see which test failed
..E/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py:643: ResourceWarning: unclosed <ssl.SSLSocket fd=5, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=(‘10.100.2.202’, 49290), raddr=(‘128.228.24.52’, 443)>
outcome.errors.clear()
……….
======================================================================
ERROR: test_is_logged_in (main.TestAPIIntegration)
———————————————————————-
Traceback (most recent call last):
File “./src/tests/tests.py”, line 233, in test_is_logged_in
api.login()
File “/usr/local/lib/python3.7/site-packages/cunyfirstapi/cunyfirstapi.py”, line 59, in login
self._password,
File “/usr/local/lib/python3.7/site-packages/cunyfirstapi/login.py”, line 66, in login
encreply = tree.xpath(‘//*[@name=”enc_post_data”]/@value’)[0]
IndexError: list index out of range


Ran 13 tests in 1.652s

FAILED (errors=1)

1 Warning
⚠️ 257 PEP 8 issues found
1 Message
📖 Thanks for remembering to add a test!

Generated by 🚫 Danger

@Huddie

Huddie commented May 31, 2019

Copy link
Copy Markdown
Collaborator

Hey looks cool but a few changes

  1. Flags should be passed always with either a true or false value.

—hides_grades=“true” for example

  1. hides_grades should not be passed into welcome_message. It should be a set global variable. Maybe even create a sessionConfig object that holds config for the session.

refresh(remaining_attempts - 1)

def start_notifier(is_welcome=False):
#global hide_grades

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

block comment should start with '# '

@Huddie Huddie left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good

@Huddie

Huddie commented Jul 1, 2019

Copy link
Copy Markdown
Collaborator

what just happened. why are so may files changed all of a sudden
what did you just change?

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.

3 participants