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

Fix indentation in exercise files #27

Merged
merged 1 commit into from
Dec 26, 2023
Merged

Conversation

pfertyk
Copy link
Contributor

@pfertyk pfertyk commented Dec 19, 2023

Mixed indentation causes some test files to not be executed correctly, and in results we're skipping tests for some exercises. Unfortunately, current CI doesn't detect these types of issues. but perhaps later we can upgrade the test runner to handle this.

@pfertyk
Copy link
Contributor Author

pfertyk commented Dec 19, 2023

@BNAndras just as I suspected, some of the tests were not running. It was caused by mixed indentation, which was not detected by the linter. I think the test runner in its current form trusts that the test files will be correct, and doesn't double-check them (as they are completely under our control, as opposed to the solution file). I will later check if the test runner can be upgraded, or if our linter can detect this.

@BNAndras
Copy link
Sponsor Member

Probably could be a supplemental GitHub action wrapped around a Bash script that checks for mixed indentation. If you don’t have the time, we can check on Discord if one of the other maintainers would be willing to make one for the track. My Bash knowledge is fairly rudimentary unfortunately.

@pfertyk
Copy link
Contributor Author

pfertyk commented Dec 19, 2023

We can check that as well ;) In general though, I would consider upgrading the runner, because some issues can still go past the linter, and it would be nice to immediately see any problems with test files.

@pfertyk
Copy link
Contributor Author

pfertyk commented Dec 21, 2023

BTW, can I get a +1 on this PR @BNAndras , if everything is OK ? :)

@BNAndras
Copy link
Sponsor Member

At least in the Github UI, it looks like you were using two tabs. Is that intentional?

I don't have write access for this repo (not a maintainer) if that's what you're looking for. You can request a review from Erik if it's saying you still need a review.

@pfertyk
Copy link
Contributor Author

pfertyk commented Dec 22, 2023

Yeah, apparently GitHub UI displays one tab as 8 spaces ... but if you select the area with your cursor, you can see it's a single character ;)

I can merge without a review (I do have such permissions), but I wanted someone else to take a look first ;) I think that, considering the amount of work you're putting into this track, that you should also be a maintainer and gain write access. @ErikSchierboom do you think this could be arranged?

@BNAndras
Copy link
Sponsor Member

No need to be a maintainer. I'm just glad to help.

@BNAndras BNAndras mentioned this pull request Dec 26, 2023
@BNAndras
Copy link
Sponsor Member

Looks like all tests are accounted for by the test runner.

@pfertyk pfertyk merged commit 057131a into main Dec 26, 2023
3 checks passed
@pfertyk pfertyk deleted the fix-indentation-in-exercise-files branch December 26, 2023 21:25
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.

None yet

2 participants