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

style: Black formatting showcase #68

Closed

Conversation

Venefilyn
Copy link
Member

Showcase of what would be changed using black. Refer to #67

@packit-as-a-service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable @oamg/convert2rhel
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@Venefilyn Venefilyn changed the title Style/formatter black showcase style: Black formatting showcase Jun 2, 2020
@zhukovgreen
Copy link
Contributor

zhukovgreen commented Jun 2, 2020

from my perspective the biggest benefit of the black formating is that we're no longer discussing a code style. It is just one, uncompromised. This saves a lot of time.
`+ black works nicely with the pre-commit

@Venefilyn
Copy link
Member Author

from my perspective the biggest benefit of the black formating is that we're no longer discussing a code style. It is just one, uncompromised. This saves a lot of time.
`+ black works nicely with the pre-commit

I agree 100%. I'm sure we will run into differences between pylint and black that need to be resolved though. Especially given we use pylint 1.9 rather than 2+

@zhukovgreen
Copy link
Contributor

from my perspective the biggest benefit of the black formating is that we're no longer discussing a code style. It is just one, uncompromised. This saves a lot of time.
`+ black works nicely with the pre-commit

I agree 100%. I'm sure we will run into differences between pylint and black that need to be resolved though. Especially given we use pylint 1.9 rather than 2+

Never tried:)

@sturivny sturivny marked this pull request as ready for review June 5, 2020 08:40
loggerinst.warning("By continuing all further changes on the system"
" will need to be reverted manually by the user,"
" if necessary.")
loggerinst.warning("The tool allows rollback of any action until this" " point.")
Copy link
Member Author

Choose a reason for hiding this comment

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

Only bug I've seen so far is the lack of handling of multi-line strings, here it doesn't merge the strings but just brings them to the same line

@fellipeh
Copy link
Contributor

@SpyTec Please resolve the conflicting files, if you still want to merge this PR.

@Venefilyn
Copy link
Member Author

@SpyTec Please resolve the conflicting files, if you still want to merge this PR.

We'll open a new PR once we've had a larger discussion with OAMG and this is something we'd want to pursue. Can remain open for now

@zhukovgreen
Copy link
Contributor

I assume next week to make a MR which will be including the black formatter as well. I’ll ping you once it is ready and, ideally, we can do absolutely the same for c2r

@zhukovgreen
Copy link
Contributor

Just an update: have to postpone the work on black formatter to the next week

@zhukovgreen
Copy link
Contributor

zhukovgreen commented Jul 28, 2020

One more update. Guys, I have to postpone the work on the feature, due to another, more important tasks. Please follow the progress here - OAMG-3458. I won't give any next date term, since already now feeling bad about this. So, I just will try to do this ASAP, since pre-commits hooks, black formatting really a huge boost to most of the devs performance

@Venefilyn
Copy link
Member Author

One more update. Guys, I have to postpone the work on the feature, due to another, more important tasks. Please follow the progress here - OAMG-3458. I won't give any next date term, since already now feeling bad about this. So, I just will try to do this ASAP, since pre-commits hooks, black formatting really a huge boost to most of the devs performance

No rush! It is relatively low priority compared to other tasks on our side as well :)

@zhukovgreen
Copy link
Contributor

ref oamg/leapp-repository#565

@Venefilyn Venefilyn marked this pull request as draft August 21, 2020 12:38
@Venefilyn Venefilyn closed this Oct 23, 2020
@Venefilyn Venefilyn mentioned this pull request Feb 9, 2021
@Venefilyn Venefilyn deleted the style/formatter-black-showcase branch October 12, 2023 15:19
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.

None yet

4 participants