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

Fixes #24283 - remove pf bindMethods function #7538

Merged
merged 1 commit into from
Aug 27, 2018
Merged

Fixes #24283 - remove pf bindMethods function #7538

merged 1 commit into from
Aug 27, 2018

Conversation

amirfefer
Copy link
Contributor

According to this, patternfly bindMethods is depracted and uses Class properties instead.

@theforeman-bot
Copy link

Issues: #24283

@johnpmitsch
Copy link
Contributor

Nice, now we won't have to see those annoying deprecation warnings :)
Can you explain the eslint changes?

@johnpmitsch johnpmitsch mentioned this pull request Jul 20, 2018
Merged
@amirfefer
Copy link
Contributor Author

@johnpmitsch , class properties is a babel experimental feature, katello has no babel parser at all, foreman does it.

@amirfefer amirfefer force-pushed the 24283 branch 2 times, most recently from 69b0639 to 2ae246c Compare July 22, 2018 12:59
@johnpmitsch
Copy link
Contributor

@amirfefer gotcha, thanks for the explanation :)

looks good, but needs a rebase

@amirfefer
Copy link
Contributor Author

Rebased.

@waldenraines
Copy link
Contributor

Rebased.

Sorry, @amirfefer looks like another rebase is needed.

waldenraines
waldenraines approved these changes Aug 3, 2018
View reviewed changes
Copy link
Contributor

@waldenraines waldenraines left a comment

Choose a reason for hiding this comment

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

LGTM

@jlsherrill
Copy link
Member

@amirfefer mind rebasing? @waldenraines, amir doesn't have merge permissions so we have to merge for him.

@waldenraines
Copy link
Contributor

@amirfefer mind rebasing? @waldenraines, amir doesn't have merge permissions so we have to merge for him.

1

Oops, I completely forgot about this PR, sorry @amirfefer!

@amirfefer
Copy link
Contributor Author

amirfefer commented Aug 26, 2018 •
edited
Loading

@waldenraines could you have another look (this time don't forget the merging stuff :) )

@waldenraines
Copy link
Contributor

[test katello]

@waldenraines
Copy link
Contributor

@amirfefer looks like there are a couple of linting issues to fix here.

@waldenraines
Copy link
Contributor

[test katello]

@waldenraines waldenraines merged commit d23e07c into Katello:master Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waldenraines waldenraines waldenraines approved these changes

Assignees
No one assigned
Labels
Needs testing Not yet reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@amirfefer @theforeman-bot @johnpmitsch @waldenraines @jlsherrill