Earlier this week, I blogged about my plans to fix a bug in a larger GitHub project. In this blog post, I will share my progress in fixing this bug.
Current Progress
By the time I made my initial blog post I identified the bug to be in the clang-format-lint GitHub Action.
Bug Location
I determined this after dozens of attempts working with the other GitHub Action, tj-actions/changed-files which feeds a list of modified files to clang-format-lint.
Planning my Bug Fix
Over the last couple of weeks, I deliberated on how to add my bugfix/feature upgrade to DoozyX/clang-format-lint. I've never worked on a GitHub Action before, and I wasn't sure how to test my changes, as this is a GitHub Action to be used within another GitHub Action. After a couple days, I consulted my class instructor for some advice.
My Instructor's Insights
After consulting my instructor, I had a better idea of how to proceed. There's still a lot I don't understand, but the GitHub Action has the following different pieces:
- a folder containing executables of supported Clang-Format releasese/versions
- a YAML file defining the GitHub Action
- a Python wrapper script that passes the GitHub Action options to the clang-format static tool
- a BASH entry point script that runs the Python script
- a Dockerfile that copies these files and runs the entry point script
How to test my changes
From my earlier work, I knew I would have to make changes to the Python script, but I wasn't sure how I could test these changes. Would I have to publish my fork of the GitHub Action and use it in another workflow?
It turns out I could just test these changes locally, by testing the Python script itself. I found that Windows 10 was unwieldy for testing, as a clang-format executable would have to be passed to the script for proper execution. Luckily, I have Ubuntu installed on my laptop for a robotics class I'm taking, and found it very useful for installing a clang-format release and testing the script.
My Changes
After learning I could just test the Python script, I promptly experimented with the Python script and made my changes. I considered a couple ways implementing my bugfix, but after consulting my instructor I opted to make minimal additions to the script. In my experiments, I found that filepaths with whitespaces are broken apart in a function that uses the String split() method. By default, this method uses all whitespaces as a separator.
For this function to support filepaths with whitespaces, I modified it to use the re.split() method to split the input string by all whitespaces except those escaped by a backslash.
Submitting an Issue
In my previous Open Source contributions, I usually make a Issue before making my code additions/changes. However, for this bug fix I didn't submit an Issue until I had a fix ready. I did this because I was worried about submitting an Issue I had no input on how to fix, but in hindsight, this might have been a bad idea. If my changes aren't accepted as my work would be in vain.
Funny enough, after making my modifications, I searched through the clang-format-lint repository to see if an Issue was already made for this feature. In my search I found a Pull Request that added support for passing multiple filepaths. In that Pull Request, the contributor noted that a limitation of the feature was that it didn't support filepaths with whitespaces, and it turned out the changes I made in my PR were upgrades to the code added in this Pull Request.
Pull Request
Shortly after submitting my Issue, I worked on and submitted my Pull Request. In it, I detailed my changes and how I tested them.
Future Plans
At this point, my Pull Request is pending review and approval by the clang-format-lint action maintainers. I'm not sure how long this will take, but I'm hoping to work with them to have these feature added.
Top comments (1)
Some comments may only be visible to logged-in visitors. Sign in to view all comments.