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

Fix #109, Move variables declared mid-function to the top #333

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Fix #109, Move variables declared mid-function to the top #333

merged 1 commit into from
Mar 9, 2023

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution
Fixes #109
All variables (that I could find) that were declared mid-function were moved to the top of their respective functions.

Testing performed
Just the CI GitHub actions so far.

Expected behavior changes
Aligns CF with the rest of cFS and the relevant coding guidelines.
Makes it easier and quicker to identify all variables in a function.

Contributor Info
@thnkslprpt

@thnkslprpt thnkslprpt force-pushed the fix-109-variable-declarations branch 2 times, most recently from a02d29f to a4e5277 Compare October 13, 2022 11:24
@thnkslprpt thnkslprpt marked this pull request as ready for review October 18, 2022 08:02
@dzbaker dzbaker requested a review from chillfig October 20, 2022 18:44
@dzbaker
Copy link
Contributor

dzbaker commented Oct 20, 2022

@thnkslprpt Can you run the Clang formatter?

@thnkslprpt thnkslprpt force-pushed the fix-109-variable-declarations branch from a4e5277 to d087112 Compare October 20, 2022 22:19
@thnkslprpt
Copy link
Contributor Author

@thnkslprpt Can you run the Clang formatter?

Yes OK - I think i got it working now.

chillfig
chillfig requested changes Oct 21, 2022
View reviewed changes
Copy link
Contributor

@chillfig chillfig left a comment

Choose a reason for hiding this comment

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

Just a nit-pick on comment placement. Thank you for the contribution!

fsw/src/cf_cfdp.c Outdated Show resolved Hide resolved
@thnkslprpt thnkslprpt force-pushed the fix-109-variable-declarations branch from d087112 to 79486ee Compare October 21, 2022 21:22
@thnkslprpt thnkslprpt requested a review from chillfig October 21, 2022 21:23
chillfig
chillfig requested changes Oct 27, 2022
View reviewed changes
Copy link
Contributor

@chillfig chillfig left a comment

Choose a reason for hiding this comment

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

Thank you! Claang formatting needed

fsw/src/cf_cfdp.c Outdated Show resolved Hide resolved
@thnkslprpt thnkslprpt force-pushed the fix-109-variable-declarations branch from 79486ee to e151abd Compare October 27, 2022 21:28
@thnkslprpt thnkslprpt requested a review from chillfig October 27, 2022 21:32
@dzbaker dzbaker added this to the Fornax milestone Nov 21, 2022
@dzbaker dzbaker modified the milestones: Fornax, Equuleus Dec 7, 2022
@thnkslprpt thnkslprpt force-pushed the fix-109-variable-declarations branch from 4e1bb97 to 9b31ad9 Compare December 26, 2022 05:24
@thnkslprpt
Fix nasa#109, Move variables declared mid-function to the top
dec4440 
commit 4e1bb97
Author: Avi Weiss <[email protected]>
Date:   Mon Dec 26 16:14:26 2022 +1100

    Typo from merge from main

commit 57ab176
Merge: e151abd 8368adc
Author: Avi <[email protected]>
Date:   Mon Dec 26 15:16:21 2022 +1100

    Merge branch 'main' into fix-109-variable-declarations

commit e151abd
Author: Avi <[email protected]>
Date:   Thu Oct 13 18:34:51 2022 +1000

    Fix nasa#109, Move variables declared mid-function to the top
@thnkslprpt thnkslprpt force-pushed the fix-109-variable-declarations branch from 9b31ad9 to dec4440 Compare December 26, 2022 05:26
@dzbaker dzbaker requested a review from chillfig March 2, 2023 19:30
chillfig
chillfig approved these changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@chillfig chillfig left a comment

Choose a reason for hiding this comment

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

Re-reviewed. Some initializations are moved along with the declaration line changes, however, these are done to variables whose assignment is unchanged whether mid-function or top.

@dzbaker dzbaker merged commit 4ce1aa9 into nasa:main Mar 9, 2023
@thnkslprpt thnkslprpt deleted the fix-109-variable-declarations branch March 9, 2023 21:02
thnkslprpt added a commit to thnkslprpt/CF that referenced this pull request Mar 9, 2023
@thnkslprpt
Squashed commit of the following:
ffc7ec5 
commit 3454b3501d05c1e538a9068572b70d2ba35329f3
Author: Avi <[email protected]>
Date:   Fri Mar 10 09:32:15 2023 +1000

    Fix nasa#105, Remove 'dummy' from variable names

commit e18bde9
Merge: 8fb8d9c cb2977c
Author: Avi <[email protected]>
Date:   Fri Mar 10 08:41:21 2023 +1000

    Merge branch 'main' into fix-105-remove-dummy-from-variable-names

commit cb2977c
Merge: 4ce1aa9 3444ff4
Author: dzbaker <[email protected]>
Date:   Thu Mar 9 15:23:53 2023 -0500

    Merge pull request nasa#366 from thnkslprpt/fix-365-remove-extraneous-assignments

    Fix nasa#365, Remove extraneous assignments to CFE_SUCCESS

commit 4ce1aa9
Merge: c44e372 dec4440
Author: dzbaker <[email protected]>
Date:   Thu Mar 9 15:20:08 2023 -0500

    Merge pull request nasa#333 from thnkslprpt/fix-109-variable-declarations

    Fix nasa#109, Move variables declared mid-function to the top

commit 8fb8d9c
Author: Avi <[email protected]>
Date:   Thu Feb 23 12:45:59 2023 +1000

    Fix nasa#105, Remove 'dummy' from variable names

commit 3444ff4
Author: Avi <[email protected]>
Date:   Thu Feb 16 10:39:06 2023 +1000

    Fix nasa#365, Remove extraneous assignments to CFE_SUCCESS

commit dec4440
Author: Avi Weiss <[email protected]>
Date:   Mon Dec 26 16:23:48 2022 +1100

    Fix nasa#109, Move variables declared mid-function to the top

    commit 4e1bb97
    Author: Avi Weiss <[email protected]>
    Date:   Mon Dec 26 16:14:26 2022 +1100

        Typo from merge from main

    commit 57ab176
    Merge: e151abd 8368adc
    Author: Avi <[email protected]>
    Date:   Mon Dec 26 15:16:21 2022 +1100

        Merge branch 'main' into fix-109-variable-declarations

    commit e151abd
    Author: Avi <[email protected]>
    Date:   Thu Oct 13 18:34:51 2022 +1000

        Fix nasa#109, Move variables declared mid-function to the top
thnkslprpt added a commit to thnkslprpt/CF that referenced this pull request Mar 10, 2023
@thnkslprpt
Squashed commit of the following:
c23dcd5 
commit 4ef9460
Author: Avi <[email protected]>
Date:   Fri Mar 10 10:10:40 2023 +1000

    Formatting

commit e18bde9
Merge: 8fb8d9c cb2977c
Author: Avi <[email protected]>
Date:   Fri Mar 10 08:41:21 2023 +1000

    Merge branch 'main' into fix-105-remove-dummy-from-variable-names

commit cb2977c
Merge: 4ce1aa9 3444ff4
Author: dzbaker <[email protected]>
Date:   Thu Mar 9 15:23:53 2023 -0500

    Merge pull request nasa#366 from thnkslprpt/fix-365-remove-extraneous-assignments

    Fix nasa#365, Remove extraneous assignments to CFE_SUCCESS

commit 4ce1aa9
Merge: c44e372 dec4440
Author: dzbaker <[email protected]>
Date:   Thu Mar 9 15:20:08 2023 -0500

    Merge pull request nasa#333 from thnkslprpt/fix-109-variable-declarations

    Fix nasa#109, Move variables declared mid-function to the top

commit 8fb8d9c
Author: Avi <[email protected]>
Date:   Thu Feb 23 12:45:59 2023 +1000

    Fix nasa#105, Remove 'dummy' from variable names

commit 3444ff4
Author: Avi <[email protected]>
Date:   Thu Feb 16 10:39:06 2023 +1000

    Fix nasa#365, Remove extraneous assignments to CFE_SUCCESS

commit dec4440
Author: Avi Weiss <[email protected]>
Date:   Mon Dec 26 16:23:48 2022 +1100

    Fix nasa#109, Move variables declared mid-function to the top

    commit 4e1bb97
    Author: Avi Weiss <[email protected]>
    Date:   Mon Dec 26 16:14:26 2022 +1100

        Typo from merge from main

    commit 57ab176
    Merge: e151abd 8368adc
    Author: Avi <[email protected]>
    Date:   Mon Dec 26 15:16:21 2022 +1100

        Merge branch 'main' into fix-109-variable-declarations

    commit e151abd
    Author: Avi <[email protected]>
    Date:   Thu Oct 13 18:34:51 2022 +1000

        Fix nasa#109, Move variables declared mid-function to the top
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chillfig chillfig chillfig approved these changes

Assignees
No one assigned
Labels
CCB:Approved
Projects
None yet
Milestone
Equuleus
Development

Successfully merging this pull request may close these issues.

Variables declared mid-function
3 participants
@thnkslprpt @dzbaker @chillfig