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 #53 - include empty header file #54

Merged

Conversation

TBladen
Copy link
Contributor

@TBladen TBladen commented Apr 3, 2018

Fix #53 by moving exception handling to the processing function, so that erroneous conditions (e.g. file not found) and an empty file (empty string) are handled differently. Includes an updated feature test case for regression testing.

@codecov-io
Copy link

codecov-io commented Apr 3, 2018

Codecov Report

Merging #54 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #54      +/-   ##
========================================
- Coverage      98%    98%   -0.01%     
========================================
  Files          26     26              
  Lines        3202   3200       -2     
========================================
- Hits         3138   3136       -2     
  Misses         64     64
Impacted Files Coverage Δ
shivyc/preproc.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25eeaf2...24b599f. Read the comment docs.

@ShivamSarodia
Copy link
Owner

Thank you for this PR! Improving the ShivyC preprocessor is a high-priority task - see #47 for more.

Feel free to amend this PR to add yourself to the Contributors list in the README (you might need to rebase to see it). Overall LGTM.

@TBladen TBladen force-pushed the bugfix/include-empty-file branch from 6d30797 to 24b599f Compare April 4, 2018 05:17
@TBladen
Copy link
Contributor Author

TBladen commented Apr 4, 2018

I've added myself and squashed the commits to make the pull request a bit cleaner. I'm definitely interested in doing more work on your project though, I was going to take a look at #37 later today :)

@ShivamSarodia
Copy link
Owner

Perfect, thanks :) I'll add some comments to #37 to describe a bit more about the preprocessor situation.

@ShivamSarodia ShivamSarodia merged commit 8b70907 into ShivamSarodia:master Apr 4, 2018
ShivamSarodia pushed a commit that referenced this pull request Apr 4, 2018
splasky pushed a commit to splasky/ShivyC that referenced this pull request Aug 5, 2019
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