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

Generated bad codes by corrupting good codes #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MiSayali
Copy link
Collaborator

Please review the error generation code.

@MiSayali MiSayali requested a review from akanshak984 December 15, 2022 10:21
gen_err.py Outdated Show resolved Hide resolved
gen_err.py Outdated
import random
import re

__action_pattern_map = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason for naming this variable with double underscores at the start?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's remove the underscores then.

gen_err.py Outdated
'replaceElifwithElseif': ("elif", "elseif"),
}

_actions = list(__action_pattern_map.keys())
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason to start this variable with underscore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's remove it from start then

gen_err.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@akanshak984 akanshak984 left a comment

Choose a reason for hiding this comment

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

Could you please test this python file to make sure the file work as expected even after the changes (like variable name change)?

gen_err.py Outdated
actions_kw = list(action_pattern_map_kw.keys())

def corrupt_syntax(line):

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a good practice to leave empty line at the start of function?

Didn't just mean to follow python code style guide for if else, but the whole code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Empty lines were just for good readability. Will update according to style guide.
Tested the code before committing changes. Code is running as per requirement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Empty lines were just for good readability.

No worries, the perspective of good readability differs from person to person.

See the usage of blank lines here for reference: https://peps.python.org/pep-0008/#blank-lines

gen_err.py Outdated
return line

def corrupt_kw_typo(line):

Copy link
Collaborator

Choose a reason for hiding this comment

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

why this empty line?

@akanshak984
Copy link
Collaborator

could you please squash the commits into one.

@MiSayali
Copy link
Collaborator Author

MiSayali commented Feb 9, 2023

Do you mean along with the first commit?

All the commits into 1 i meant.

@akanshak984
Copy link
Collaborator

Do you mean along with the first commit?

All the commits into 1 i meant.

Yes!

@akanshak984
Copy link
Collaborator

akanshak984 commented Feb 13, 2023

Merging this for now.
Make sure in future PR's we only have relevant commits.

For example:
Let's say there is a commit A in which you have added a function
And in commit B you have modified that function.

Now, in master branch.
If something goes wrong in this function - you will be required to check only one commit. If we have squashed them into one.
Othervise you will need to check two commits.

So, to keep main branch clean with relevant commits. we squash commits which aren't atomic and independent on their own.

@MiSayali
Copy link
Collaborator Author

Okay
Got it.

Removed underscores and extra lines.
Followed python code convention.

Generated bad codes by corrupting good codes
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.

2 participants