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

Fix formatter to add a new line after the preprocessor directives part #17

Closed
wants to merge 2 commits into from
Closed

Fix formatter to add a new line after the preprocessor directives part #17

wants to merge 2 commits into from

Conversation

keyhr
Copy link
Collaborator

@keyhr keyhr commented Dec 31, 2022

Fix for dawnbeen#38

@keyhr keyhr changed the title Fix for dawnbeen#38 Fix formatter to add a new line after the preprocessor directives part Dec 31, 2022
@keyhr
Copy link
Collaborator Author

keyhr commented Dec 31, 2022

I'm not sure why the test fails with python 3.6 on ubuntu-latest

@keyhr keyhr requested a review from cacharle December 31, 2022 07:34
@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

I have no idea why the CI is failing, I looked in the file where there is the python versions (from the CI logs) and there is v3.6 in there.

I tried to rerun but didn't work.

@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

Also, the main repo has the pypi api key now, so u can make your PRs there.

I asked @dawnbeen to make you contributor so you're more free if you want to make more contributions.

@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

I just looked it up and Python3.6 has reached end of life a year ago so we can just drop it from the CI.

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

I asked @dawnbeen to make you contributor so you're more free if you want to make more contributions.

OK, sorry to bother you. Should I move this PR to this repo?

@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

It's no problem, I'm glad you're fixing issues.

I don't know if you can transfer PRs cleanly.. Otherwise it's probably easier for both you and me if you delete it here and open a new one in the main repo (I'll probably delete this fork anyway)

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

I have no idea why the CI is failing, I looked in the file where there is the python versions (from the CI logs) and there is v3.6 in there.
I tried to rerun but didn't work.

I just looked it up and Python3.6 has reached end of life a year ago so we can just drop it from the CI.

That makes sense... I didn't know that Python3.6 no longer being supported.

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

Saying "move this PR", I meant opening a new one on the main repo. (I don't know any other way to do it...)

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

By the way, I'm no longer 42 student anymore too, so I contribute when I feel like it. Perhaps I will ask for some review again.

@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

By the way, I'm no longer 42 student anymore too, so I contribute when I feel like it. Perhaps I will ask for some review again.

Yeah, don't worry

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

Do you think these commits fix the issue?

@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

Looks good, you added tests and didn't break other tests

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

OK, thanks.

It looks that I don't have 'write' permission on the main branch. I've just made PR but cannot merge it. I'm not sure how to make changes to the main repo...

@cacharle
Copy link
Owner

cacharle commented Jan 2, 2023

Yeah, I asked @dawnbeen by mail 5min ago so you probably don't have access right now 😅 but you'll receive a notification of mail as soon as she gives you access (I can't give you access myself)

I'll merge your stuff in the main branch in the meantime.

@keyhr
Copy link
Collaborator Author

keyhr commented Jan 2, 2023

"A watched pot never boils" is very very right for me 😇
Thank you very much.

@cacharle cacharle closed this Jan 2, 2023
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