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

Refactoring variables #8528

Merged
merged 6 commits into from
May 17, 2021
Merged

Refactoring variables #8528

merged 6 commits into from
May 17, 2021

Conversation

jppgomes
Copy link
Contributor

@jppgomes jppgomes commented Apr 22, 2021 •
edited by indam23
Loading

Proposed changes:

@CLAassistant
Copy link

CLAassistant commented Apr 22, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@sara-tagger
Copy link
Collaborator

Thanks for submitting a pull request 🚀 @melindaloubser1 will take a look at it as soon as possible ✨

indam23
indam23 suggested changes Apr 23, 2021
View reviewed changes
Copy link
Contributor

@indam23 indam23 left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! Can you address the undefined name errors first?

@jppgomes
Copy link
Contributor Author

@melindaloubser1 yes, of course i'll fix this

indam23
indam23 reviewed Apr 29, 2021
View reviewed changes
Copy link
Contributor

@indam23 indam23 left a comment

Choose a reason for hiding this comment

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

Besides the changes in black version, this looks good!

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@indam23 indam23 self-requested a review May 3, 2021 16:23
@TyDunn TyDunn added this to the 2.6 Rasa Open Source milestone May 5, 2021
@TyDunn
Copy link
Contributor

TyDunn commented May 5, 2021

@melindaloubser1 Can we this merged in time for Rasa Open Source 2.6 tomorrow?

@TyDunn TyDunn mentioned this pull request May 5, 2021
Closed
4 tasks
@indam23
Copy link
Contributor

indam23 commented May 5, 2021

Sure I'll resolve the conflicts now

@indam23 indam23 enabled auto-merge May 5, 2021 10:01
@indam23
Copy link
Contributor

indam23 commented May 5, 2021

@jppgomes I'm not able to update your branch with the merge conflict resolution, can you resolve it please?

@jppgomes
Copy link
Contributor Author

jppgomes commented May 5, 2021

@melindaloubser1 yes!!

auto-merge was automatically disabled May 5, 2021 12:01

Head branch was pushed to by a user without write access

@indam23 indam23 enabled auto-merge May 5, 2021 12:04
auto-merge was automatically disabled May 5, 2021 12:34

Head branch was pushed to by a user without write access

@indam23 indam23 enabled auto-merge May 5, 2021 13:44
@joejuzl joejuzl removed this from the 2.6 Rasa Open Source milestone May 6, 2021
@indam23 indam23 merged commit f1e19f5 into RasaHQ:main May 17, 2021
@ErickGiffoni ErickGiffoni deleted the refactoring_clean_code branch August 5, 2021 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@indam23 indam23 indam23 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Variables with a non expressive name and refactoring method. Non expressive variable name on domain.py
6 participants
@jppgomes @CLAassistant @sara-tagger @TyDunn @indam23 @joejuzl