Skip to content

multi-apply-tool: fix some syntax errors#436

Merged
nephros merged 5 commits intosailfishos-patches:masterfrom
nephros:fix-tool
May 30, 2023
Merged

multi-apply-tool: fix some syntax errors#436
nephros merged 5 commits intosailfishos-patches:masterfrom
nephros:fix-tool

Conversation

@nephros
Copy link
Contributor

@nephros nephros commented May 26, 2023

It seems the history of #302 ended up in some merge/syntax errors

These are some quick changes to make the script work again.

@nephros nephros added the debt fallout and other issues originating from the past label May 26, 2023
@nephros nephros requested a review from Olf0 May 26, 2023 09:18
Olf0
Olf0 previously approved these changes May 27, 2023
Copy link
Contributor

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

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

Have to review the export case again, when less tired.

P.S.: Oops, this was my mishap(s) starting with commit f43e8b6 .

@Olf0 Olf0 dismissed their stale review May 27, 2023 02:40

Have to review the export case again, when less tired.

Copy link
Contributor

@b100dian b100dian left a comment

Choose a reason for hiding this comment

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

LGTM. The changes are quite clear with white-space ignoring diff.

@nephros nephros merged commit ae85a89 into sailfishos-patches:master May 30, 2023
@Olf0
Copy link
Contributor

Olf0 commented Jun 7, 2023

Yes, these changes were fine, but the double shift in lines 75 & 76 is not, and the additional check for sufficient parameters should also be introduced to ´-a´ and ´-d´. Edit: Nope, the latter is fine, Patchlist is checked for being empty later on.

Will do, hopefully tonight. Edit: Done by #438.

Copy link
Contributor

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

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

All fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debt fallout and other issues originating from the past

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants