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

Support include files with dot in name #125

Merged
merged 3 commits into from
Aug 19, 2020
Merged

Support include files with dot in name #125

merged 3 commits into from
Aug 19, 2020

Conversation

aisk
Copy link
Member

@aisk aisk commented Mar 3, 2020

Not a completed work, for now just support "one level include", like including a file called "com.example.api.thrift".

On the opposite, including a file called "com.example.thrift", which also including another file "api.thrift", is not supported now.

So this PR is marked as WIP, I'll try to finish this latter.

Close #121

@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

Merging #125 into master will decrease coverage by 3.32%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
- Coverage   83.09%   79.77%   -3.33%     
==========================================
  Files          43       43              
  Lines        3911     3915       +4     
==========================================
- Hits         3250     3123     -127     
- Misses        661      792     +131     
Impacted Files Coverage Δ
thriftpy2/parser/parser.py 95.57% <94.11%> (+0.09%) ⬆️
thriftpy2/contrib/tracking/__init__.py 60.71% <0.00%> (-35.74%) ⬇️
thriftpy2/server.py 37.83% <0.00%> (-27.03%) ⬇️
thriftpy2/thrift.py 77.06% <0.00%> (-11.83%) ⬇️
thriftpy2/http.py 78.73% <0.00%> (-8.28%) ⬇️
thriftpy2/contrib/tracking/tracker.py 78.57% <0.00%> (-8.17%) ⬇️
thriftpy2/tornado.py 82.60% <0.00%> (-2.58%) ⬇️
thriftpy2/contrib/aio/processor.py 82.00% <0.00%> (-2.32%) ⬇️
thriftpy2/contrib/aio/socket.py 77.47% <0.00%> (-0.55%) ⬇️

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 b981e8c...62dd565. Read the comment docs.

@aisk aisk changed the title WIP: Support include files with dot in name wip: Support include files with dot in name Mar 3, 2020
@aisk aisk changed the title wip: Support include files with dot in name WIP Support include files with dot in name Mar 3, 2020
@aisk
Copy link
Member Author

aisk commented Mar 3, 2020

The WIP label does not work, I found GitHub do not support this functionallity by default, but requires a app to be installed: https://github.com/marketplace/wip/ .

@ethe do you mind install it when you have time, I think the free plan is enough for open sourced project?

@ethe
Copy link
Member

ethe commented Mar 3, 2020

LGTM, I installed it.

@aisk aisk changed the title WIP Support include files with dot in name Support include files with dot in name Jun 14, 2020
@aisk
Copy link
Member Author

aisk commented Jun 14, 2020

Sorry I most forgot this PR. Although I think this PR is incomplete for implemented all the features, but this does fit what we need in company, and we run the codes in this PR for monthes (from when this PR created). So I think we can just merge it.

@ethe what do you think?

@ethe ethe merged commit a8b7873 into master Aug 19, 2020
@windard windard mentioned this pull request Oct 10, 2020
@ethe ethe deleted the issue-121 branch November 10, 2022 11:32
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.

include with dot in string
2 participants