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.

Sign up for GitHub

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

Jump to bottom

CLI: sync report logic \w Linguist #214

Merged
merged 9 commits into from
Apr 9, 2019
Merged

CLI: sync report logic \w Linguist #214

merged 9 commits into from
Apr 9, 2019

Conversation

bzz
Copy link
Contributor

@bzz bzz commented Apr 3, 2019 •
edited
Loading

Fixes #204 by adjusting Enry CLI app behavior according to Linguist upstream.

bzz added 4 commits April 3, 2019 15:40
@bzz
gen: refactoring, renaming vars for readability
b2b61c2 
This does not change the logic of the generatro
but only renames/moves some vars for readability

Signed-off-by: Alexander Bezzubov <[email protected]>
@bzz
doc: better wording in API godoc
df01124 
Signed-off-by: Alexander Bezzubov <[email protected]>
@bzz
cli: mimic linguist output by default
88810fe 
This includes next main changes:

 - default: print only Programming and Markup types
   as Linguist does
 - `-prog` option replaced with `-all`, to allow for
   previous behavior
 - always use GetLanguage as main source of truth
   that fixes src-d#204 and perf will be restored under src-d#212

Signed-off-by: Alexander Bezzubov <[email protected]>
@bzz
doc: update godoc and README \w supported features
c9f1793 
Signed-off-by: Alexander Bezzubov <[email protected]>
@bzz bzz self-assigned this Apr 3, 2019
@bzz bzz changed the title CLI app: sync accuracy of the report \w Linguist WIP CLI app: sync accuracy of the report \w Linguist Apr 3, 2019
@bzz
doc: update TravisCI links
94e8598 
Signed-off-by: Alexander Bezzubov <[email protected]>
@bzz bzz added this to the v1.8.0 milestone Apr 4, 2019
@bzz bzz mentioned this pull request Apr 4, 2019
Closed
@bzz bzz changed the title WIP CLI app: sync accuracy of the report \w Linguist WIP CLI: sync report logic \w Linguist Apr 4, 2019
@bzz bzz changed the title WIP CLI: sync report logic \w Linguist CLI: sync report logic \w Linguist Apr 4, 2019
@bzz bzz requested review from dennwc and creachadair April 4, 2019 20:03
@bzz
Copy link
Contributor Author

bzz commented Apr 4, 2019 •
edited
Loading

Here is the output of enry from this PR and linguist 7.3.0

$ enry /tmp/linguist-django/django/
95.86%	Python
1.85%	JavaScript
1.65%	HTML
0.63%	CSS
0.01%	Shell
0.00%	Smarty
0.00%	Makefile

$ linguist /tmp/linguist-django/django
95.86%  Python
1.85%   JavaScript
1.65%   HTML
0.63%   CSS
0.01%   Shell
0.00%   Smarty
0.00%   Makefile

\cc @creachadair @dennwc it's ready to be reviewed.
As it includes multiple fixes, most probably it easier to do that per-commit.

@bzz bzz mentioned this pull request Apr 4, 2019
Closed
creachadair
creachadair reviewed Apr 4, 2019
View reviewed changes
Copy link
Contributor

@creachadair creachadair left a comment

Choose a reason for hiding this comment

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

A few minor suggestions and a couple questions, but generally this looks good to me.

internal/code-generator/generator/samplesfreq.go Outdated Show resolved Hide resolved
internal/code-generator/generator/samplesfreq.go Outdated Show resolved Hide resolved
internal/code-generator/generator/samplesfreq.go Outdated Show resolved Hide resolved
cmd/enry/main.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cmd/enry/main.go Outdated Show resolved Hide resolved
cmd/enry/main.go Outdated Show resolved Hide resolved
cmd/enry/main.go Outdated Show resolved Hide resolved
common.go Outdated Show resolved Hide resolved
internal/code-generator/generator/samplesfreq.go Outdated Show resolved Hide resolved
@bzz bzz modified the milestones: v1.8.0, v1.7.3 Apr 8, 2019
@bzz
Copy link
Contributor Author

bzz commented Apr 8, 2019

@creachadair thank you for detailed review! All feedback was addressed and it's ready for another round.

Meanwhile, I'll going to look into CI failures.

@bzz bzz requested a review from creachadair April 8, 2019 14:14
@bzz
Copy link
Contributor Author

bzz commented Apr 8, 2019 •
edited
Loading

CI failure looks irrelevant - they did pass before but then, after the doc update it started to fail on multiple runs on go 1.11 profile.

/home/travis/.gimme/versions/go1.11.7.linux.amd64/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/bin/ld: /tmp/go-link-811669485/000006.o: unrecognized relocation (0x2a) in section `.text'
/usr/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status

If no objections, going to fixing those in a subsequent PRs.

creachadair
creachadair approved these changes Apr 8, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
@bzz bzz merged commit 7a6e8ca into src-d:master Apr 9, 2019
@bzz bzz deleted the fix-cli-accuracy branch April 9, 2019 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@creachadair creachadair creachadair approved these changes

@dennwc dennwc Awaiting requested review from dennwc

Assignees

@bzz bzz

Labels
None yet
Projects
None yet
Milestone
v1.7.3
Development

Successfully merging this pull request may close these issues.

CLI: -mode=line reports bad results Breakdown of django/django is different from Linguist
2 participants
@bzz @creachadair