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

Cleanup/Formatting #93

Merged
merged 12 commits into from
Jun 5, 2023
Merged

Cleanup/Formatting #93

merged 12 commits into from
Jun 5, 2023

Conversation

stylesuxx
Copy link
Contributor

No description provided.

@stylesuxx stylesuxx marked this pull request as draft May 20, 2023 23:35
@stylesuxx stylesuxx mentioned this pull request May 20, 2023
@stylesuxx
Copy link
Contributor Author

@damosvil Have a look - the last commit is the "clean", formatted code after running through the script. There is some stuff that I still want to clean up manually, but how do you like the general formatting? I can adjust a lot of parameters, just let me know which parts you would like to be different and I'll adjust it accordingly.

@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from 7d0bdfc to ffd6369 Compare May 21, 2023 14:07
@stylesuxx
Copy link
Contributor Author

@damosvil - alright, new iteration with 4 spaces indentation

@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from ffd6369 to a365124 Compare May 21, 2023 14:15
@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from a365124 to ac26a0f Compare May 21, 2023 17:06
@stylesuxx
Copy link
Contributor Author

stylesuxx commented May 21, 2023

Alright, here is another iteration.

There are a couple things I need to sanitize beforehand since it interferes with the other rules:

  1. Only banner comments before labels, preferably no inline comments with labels
; Comm phase 6 to comm phase 1
comm6_comm1:                            ; C->B
    jb   Flag_Motor_Dir_Rev, comm6_comm1_rev

    clr  IE_EA
    A_Pwm_Fet_Off

I would instead refactor to:

;**** **** **** **** **** **** **** **** **** **** **** **** ****
;
; Comm phase 6 to comm phase 1
; C->B
;
;**** **** **** **** **** **** **** **** **** **** **** **** ****
comm6_comm1:
    jb   Flag_Motor_Dir_Rev, comm6_comm1_rev

    clr  IE_EA
    A_Pwm_Fet_Off
  1. Banner labels are inconsistent - I would refactor them all to this form:
;**** **** **** **** **** **** **** **** **** **** **** **** ****
;[EMPTY LINE]
;[SPACE]content comes
;[SPACE]here 
;[EMPTY LINE]
;**** **** **** **** **** **** **** **** **** **** **** **** ****

this is the variant you added when splitting up module, IMHO this is the best, most readable variant and we should have that consistently.

@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from ac26a0f to 9106a37 Compare May 21, 2023 19:00
@stylesuxx
Copy link
Contributor Author

Alright, so I gave it a pass with a bit of manual re-organisation here: d741076

The latest version I applied the formatter on the sanitized version.

@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from 9106a37 to 151ae95 Compare May 23, 2023 20:54
@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from 151ae95 to cf99abe Compare May 24, 2023 13:48
@stylesuxx
Copy link
Contributor Author

Alright @damosvil this version is now with touching the comments as little as possible but still keeping some consistency. I think we would need to go through the comments manually and clean them up properly.

I would like to establish proper formatting for Block and Banner comments and any other comment type we want to have, I started an issue here: #96

I think we need to re-work the comments once we established a ruleset we want to follow. I would thus not implement any more comment formatting than is already implemented and do the rest manually after the initial pass and disable comment parsing/handling after the initial cleanup pass with the formatter.

@damosvil
Copy link
Contributor

damosvil commented Jun 2, 2023

I like the comment formatting you propose, in #96
Ok, lets do the initial pass and then update the comments manually.

@stylesuxx
Copy link
Contributor Author

@damosvil are you happy with the current state of formatting, or is there anything else I should change. If this is fine now as it is. I'd say we move forward with improving comment formatting.

@damosvil
Copy link
Contributor

damosvil commented Jun 5, 2023

Yes I like how It is. Please, lets move forward

@stylesuxx stylesuxx force-pushed the feature/v0.20-cleanup branch from 405c52e to 771936c Compare June 5, 2023 16:24
@stylesuxx stylesuxx marked this pull request as ready for review June 5, 2023 21:27
@stylesuxx stylesuxx requested a review from damosvil June 5, 2023 21:27
@stylesuxx stylesuxx linked an issue Jun 5, 2023 that may be closed by this pull request
7 tasks
@stylesuxx stylesuxx merged commit e869e3f into v0.20 Jun 5, 2023
@stylesuxx stylesuxx added the chore label Jun 5, 2023
@stylesuxx stylesuxx modified the milestones: v0.21.0, v0.20.0 Jun 15, 2023
@stylesuxx stylesuxx deleted the feature/v0.20-cleanup branch July 2, 2023 13:30
@stylesuxx stylesuxx mentioned this pull request Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup
2 participants