Skip to content

Add CS and update to upstream 1.2.6#8

Merged
Kdecherf merged 3 commits intofossar:masterfrom
Kdecherf:cs-upstream
Jan 22, 2022
Merged

Add CS and update to upstream 1.2.6#8
Kdecherf merged 3 commits intofossar:masterfrom
Kdecherf:cs-upstream

Conversation

@Kdecherf
Copy link

No description provided.

@jtojnar
Copy link
Member

jtojnar commented Nov 20, 2021

Thanks, looks good. I can also add you as an admin if you wish. cc @j0k3r

@j0k3r
Copy link

j0k3r commented Nov 20, 2021

Won't this be hard to keep the projet update with the upstream if we apply PHP-CS-Fixer?

@jtojnar
Copy link
Member

jtojnar commented Nov 20, 2021

Updating should not be a problem – just overwrite the code with fresh upstream one, run php-cs-fixer, and then re-apply our patches that have not been upstreamed yet.

Pushing changes upstream will be harder. Perhaps we could try to convince upstream to use code formatting to make that easier?

@Kdecherf
Copy link
Author

I manually applied changes shown by a simple diff against archives from sourceforge (diff -u {1.2.4.2,1.2.6}/htmLawed.php). Not ideal but it seems that there's a very low activity on upstream anyway.

@Kdecherf
Copy link
Author

... and fixing things (outside of upstream patches) is quite horrible without some CS

@Kdecherf
Copy link
Author

@jtojnar fine for me for the admin permission :)

@j0k3r
Copy link

j0k3r commented Nov 22, 2021

@Kdecherf maybe you can add a GitHub workflow to check CS for PR?

@Kdecherf
Copy link
Author

@j0k3r I've added a workflow but it seems that it will run only after being merged in master

Copy link

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Looks great, you can squash 👍

Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
@Kdecherf Kdecherf merged commit ed284a2 into fossar:master Jan 22, 2022
@Kdecherf Kdecherf deleted the cs-upstream branch January 22, 2022 18:12
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.

3 participants