Skip to content

Commit 7b9029e

Browse files
authored
Merge pull request #28 from OpenNewsLabs/develop
Merge in changes for v1.0
2 parents 67190a6 + e7c155a commit 7b9029e

File tree

8 files changed

+272
-132
lines changed

8 files changed

+272
-132
lines changed

README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ The project started as a way to automate
99
[this checklist for newsroom developers](https://docs.google.com/document/d/1kTtHAgzlyteODMia1JmIGbKkjGugxKMZfxoWEGdku_Q/edit#),
1010
but these are good practices for most open source projects!
1111

12+
This is written in and supported on Python 3.
13+
1214
##Getting Started
1315
###Installation
1416
It is recommended that you use a virtual environment in order to avoid
@@ -63,6 +65,13 @@ repository, file a pull request to get your contribution into the main
6365
repository.
6466

6567
##Changelog
68+
###version 1.0
69+
* Update `ROADMAP.md` with pre-NICAR status
70+
* Fix the KeyError issue when default config was changed ([#27](https://github.com/OpenNewsLabs/open-project-linter/issues/27))
71+
* Split some of the main linter logic out into functions and improve module
72+
documentation
73+
* Add test fixtures for the git-related tests ([#22](https://github.com/OpenNewsLabs/open-project-linter/issues/22))
74+
6675
###version 0.4dev
6776
* Now installs Pygments automatically when installed with pip, with or
6877
without pulling versions from `requirements.txt`

ROADMAP.md

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,43 +15,55 @@ want to volunteer to contribute, comment on an issue you're interested in and
1515
ping @brainwane.
1616

1717
## Milestones
18-
Version 1.0 of this tool is due to be finished in mid-February, so that it can
19-
be tested ahead of being recommended and promoted at [NICAR](https://www.ire.org/conferences/nicar2017/).
20-
### Short-term
21-
These will be finished by 16 Feb 2017.
22-
23-
Currently in progress:
24-
* Write a README documenting package purpose, installation, and usage.
25-
* Write tests for currently available functionality, beginning with
26-
rules
27-
* Package the application so it is installable through pip; keep `requirements.txt`
28-
up to date. (#1)
29-
* Publish project on PyPI (#2)
18+
Version 1.0 of this tool will be recommended and promoted at
19+
[NICAR](https://www.ire.org/conferences/nicar2017/).
3020

31-
Not currently in progress:
32-
* Refactor to add results to a result object as checks are run instead of only
33-
printing to stdout.
34-
* Implement code detection (is code in the repository?) (#9)
21+
### Short-term
22+
* Fix any bugs that come up during NICAR
3523

3624
### Medium-term
37-
These could be worked on now, but are not in progress. They may or may not be
38-
finished by 15 Feb 2017.
25+
These are the highest-priority additions and improvements.
3926

40-
* Improve the message text for help and other interface messages
41-
* Ensure that interface messages are stored together and are easy to find
42-
and modify
27+
#### Features
4328
* Add functionality/rules to check for secrets, passwords, keys, and personally
44-
identifiable information (#17)
29+
identifiable information ([#17](https://github.com/OpenNewsLabs/open-project-linter/issues/17))
4530
* Add functionality/rules to check for offensive words in code, comments, and
46-
documentation (#18)
47-
* Add functionality/rules to check whether a git commit is signed with GPG (#19)
48-
* Add functionality/rules to check whether a package is signed with GPG (#20)
49-
50-
### Long-term
51-
These are components that could be added in the future, but are not part of
52-
current development efforts. They involve inessential improvements or more
53-
involved work.
54-
* Add the ability to check a repository via the Github URL (#6)
55-
* Add the ability to check that a Github project is using milestones (#16)
31+
documentation ([#18](https://github.com/OpenNewsLabs/open-project-linter/issues/18))
32+
* Add functionality/rules to check whether a git commit is signed with GPG ([#19](https://github.com/OpenNewsLabs/open-project-linter/issues/19))
33+
* Add functionality/rules to check whether a package is signed with GPG ([#20](https://github.com/OpenNewsLabs/open-project-linter/issues/20))
34+
35+
#### Code Improvements
36+
* Refactor to add results to a result object as checks are run instead of only
37+
printing to stdout
38+
* Ensure that interface messages are stored together and are easy to find
39+
and modify
40+
41+
### Ideas for the future
42+
Future maintainers and community should decide what their priorities are,
43+
but here are some possible improvements. These are components that could be
44+
added in the future, but are not part of current development efforts. They
45+
involve inessential improvements or more involved work.
46+
47+
#### Features
48+
* Add the ability to check a repository via the Github URL ([#6](https://github.com/OpenNewsLabs/open-project-linter/issues/6))
49+
* Add the ability to check that a Github project is using milestones ([#16](https://github.com/OpenNewsLabs/open-project-linter/issues/16))
5650
* Automated license detection
51+
52+
#### Interface improvements
53+
* Improve console string formatting
54+
* Consider which options belong in the config file vs. the command-line
55+
interface
56+
* Improve the message text for help and other interface messages
57+
* Add a verbosity option (e.g. show all output vs. show only errors)
5758
* Save the results to a file instead of only printing in the terminal
59+
60+
#### Code improvements/technical debt
61+
* Improve automated test coverage
62+
* Make compatible with Python 2.7
63+
* Make compatible with Windows
64+
* Use path/Path functions for all filepath manipulations (not just strings)
65+
* As the number of rules grows, split functions out into separate submodules
66+
* As the number of automated tests grows, split them into files containing
67+
similar tests
68+
* Consider separating functions that support rules/linter implementation (that
69+
do not implement the main logic) into a utils submodule

openlinter/openlinter.py

Lines changed: 158 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,27 @@
22
"""
33
openlinter.py
44
5-
Early implementation for an open source project linter.
5+
Console entry point for the openlinter package.
66
7-
See https://github.com/OpenNewsLabs/open-project-linter/issues/21 .
7+
Functions
8+
---------
9+
main
10+
Main function of the linter--set up state and call checks.
11+
12+
get_current_script_dir
13+
Inspect the stack to find the directory the current module is in.
14+
15+
parse_linter_args
16+
Parse command-line arguments and set up the console interface.
17+
18+
get_rule_set
19+
Read and parse the configuration file.
20+
21+
check_for_git_branches
22+
Call the checks related to git branching and print the results.
23+
24+
check_multiple_git_commits
25+
Call the check for multiple commits per branch and print the result.
826
"""
927

1028
from __future__ import absolute_import
@@ -18,43 +36,22 @@
1836
import openlinter.rules as rules
1937

2038

21-
def get_current_script_dir():
22-
module_location = os.path.abspath(inspect.stack()[0][1])
23-
return os.path.dirname(module_location)
24-
25-
26-
def parse_linter_args():
27-
parser = argparse.ArgumentParser()
28-
group = parser.add_mutually_exclusive_group()
29-
group.add_argument('-d', '--directory', help="The local path to your repository's base directory. Defaults to the current working directory.",
30-
default=os.getcwd()
31-
)
32-
parser.add_argument('-r', '--rules', help='The path to the rules configuration file, a YAML file containing the rules you would like to check for. Defaults to current-working-directory/openlinter/rules.yml.',
33-
default=os.path.join(get_current_script_dir(), 'rules.yml')
34-
)
35-
parser.add_argument('-v', '--version', action='version', version='0.4dev')
36-
return parser.parse_args()
37-
38-
39-
# FIXME: this is all not very functional or testable.
39+
# FIXME: this could be more functional and testable.
4040
def main():
41-
# Set up parser and CLI
41+
# Get command-line args and configuration data
4242
args = parse_linter_args()
43-
44-
# Read in rules
45-
with open(args.rules, 'r') as f:
46-
text = f.read()
47-
48-
rule_set = yaml.safe_load(text)
43+
rule_set = get_rule_set(args)
4944

5045
# Check directory/repository against rules
46+
#######
47+
# TODO: Consider architecture: how to handle the result (print to stdout
48+
# as we go, or pass around a result object?), how to handle interface
49+
# strings (hard-coded or able to change/localize easily).
50+
#######
51+
52+
# Check for the presence of specified files
5153
if 'files_exist' in rule_set:
52-
"""
53-
# TODO: Consider architecture: how to handle the result (print to stdout
54-
# as we go, or pass around a result object?), how to handle interface
55-
# strings (hard-coded or able to change/localize easily).
56-
"""
57-
# FIXME: also unfortunately nested
54+
# FIXME: also unfortunately nested, breaking out functions will help
5855
for files_to_check in rule_set['files_exist']:
5956
for f in files_to_check:
6057
for name in files_to_check[f]:
@@ -65,16 +62,14 @@ def main():
6562
print(output)
6663
break
6764
# Otherwise note that none of the names exist?
68-
# TODO: could wrangle this a bit
69-
# brainwane/sumanah: should this say "no $name type file found"
70-
# or list the individual ones not present?
7165
elif result is None:
7266
output = '! {} exists but is empty'.format(name,
7367
args.directory)
7468
else:
7569
output = '! {} not found in {}'.format(name, args.directory)
7670
print(output)
7771

72+
# Check for the presence of any code
7873
if 'code_exists' in rule_set:
7974
code_exists = rules.check_for_code(args.directory)
8075
if code_exists:
@@ -83,6 +78,7 @@ def main():
8378
output = '! no code files found'
8479
print(output)
8580

81+
# Check for the presence of version control and git repo features
8682
if 'version_control' in rule_set:
8783
vcs = rules.detect_version_control(args.directory)
8884
if vcs:
@@ -91,45 +87,138 @@ def main():
9187
output = '! version control system not detected'
9288
print(output)
9389

94-
# Conditionals may need work here, TODO
95-
if 'detect_git_branches' in rule_set['version_control'] and vcs == 'git':
96-
branches = rules.check_multiple_branches(args.directory)
97-
if branches:
98-
output = ' multiple git branches found'
90+
# Might be better with a try/except with git.InvalidGitRepositoryError
91+
if 'detect_git_branches' in rule_set['version_control'] and vcs == 'git':
92+
git_branch_info = check_for_git_branches(args, rule_set)
93+
elif 'detect_git_branches' in rule_set['version_control']:
94+
print('! no git repository detected, could not check for git branches')
9995
else:
100-
output = '! fewer than 2 git branches found'
101-
print(output)
96+
pass
10297

103-
# check for a dev/feature branch
104-
# TODO: pull some of this out into a function
105-
develop = False
106-
for name in rule_set['dev_branch_names']:
107-
if rules.check_for_develop_branch(args.directory, name):
108-
develop = True
109-
output = ' development branch "{}" found'.format(name)
110-
print(output)
111-
if not develop:
112-
output = '! no development branch found'
113-
print(output)
114-
115-
# Conditionals may need work here, TODO
116-
if 'multiple_git_commits' in rule_set['version_control'] and vcs == 'git':
117-
multiple_commits = rules.check_for_multiple_commits(args.directory)
118-
if multiple_commits:
119-
output = ' multiple commits on a branch found'
98+
if 'multiple_git_commits' in rule_set['version_control'] and vcs == 'git':
99+
# Currently doesn't return anything
100+
multiple_commits = check_multiple_git_commits(args)
101+
elif 'multiple_git_commits' in rule_set['version_control']:
102+
print('! no git repository detected, could not check for multiple commits')
120103
else:
121-
output = '! one or fewer commits on each branch'
122-
print(output)
104+
pass
123105

124-
elif 'detect_git_branches' in rule_set['version_control']:
125-
print('! no git repository detected, could not check for git branches')
106+
#######
107+
# Checks with new rules get added here
108+
#######
126109

110+
111+
def get_current_script_dir():
112+
"""Inspect the stack to find the directory the current module is in.
113+
114+
Returns
115+
-------
116+
string
117+
a string containing the path to the module location
118+
"""
119+
module_location = os.path.abspath(inspect.stack()[0][1])
120+
return os.path.dirname(module_location)
121+
122+
123+
def parse_linter_args():
124+
"""Parse command-line arguments and set up the command-line
125+
interface and help.
126+
127+
Defaults to current working directory for the directory arg and the
128+
copy of rules.py in the directory this module is in for the rules arg.
129+
130+
Returns
131+
-------
132+
argparse.Namespace
133+
object subclass with arguments as attributes for each given argument
134+
"""
135+
parser = argparse.ArgumentParser()
136+
group = parser.add_mutually_exclusive_group()
137+
group.add_argument('-d', '--directory', help="The local path to your repository's base directory. Defaults to the current working directory.",
138+
default=os.getcwd()
139+
)
140+
parser.add_argument('-r', '--rules', help='The path to the rules configuration file, a YAML file containing the rules you would like to check for. Defaults to path/to/openlinter/rules.yml.',
141+
default=os.path.join(get_current_script_dir(), 'rules.yml')
142+
)
143+
parser.add_argument('-v', '--version', action='version', version='1.0')
144+
return parser.parse_args()
145+
146+
147+
def get_rule_set(args):
148+
"""Read and parse the config file at the location given when the
149+
linter script is run.
150+
151+
Parameters
152+
----------
153+
args : argparse.Namespace
154+
Arguments input when script is called from the console
155+
156+
Returns
157+
-------
158+
dict
159+
Contains structured data from the parsed configuration file
160+
"""
161+
with open(args.rules, 'r') as f:
162+
text = f.read()
163+
rule_set = yaml.safe_load(text)
164+
return rule_set
165+
166+
167+
def check_for_git_branches(args, rule_set):
168+
"""Call the checks related to git branching (multiple branches and
169+
appropriately named develop/feature branch) and print the results
170+
to stdout.
171+
172+
Parameters
173+
----------
174+
args : argparse.Namespace
175+
Arguments input when script is called from the console
176+
rule_set : dict
177+
Contains the structured data from the parsed configuration file
178+
179+
Returns
180+
-------
181+
None
182+
"""
183+
branches = rules.check_multiple_branches(args.directory)
184+
if branches:
185+
output = ' multiple git branches found'
127186
else:
128-
pass
187+
output = '! fewer than 2 git branches found'
188+
print(output)
189+
190+
# check for a dev/feature branch
191+
# TODO: pull some of this out into a function
192+
develop = False
193+
for name in rule_set['dev_branch_names']:
194+
if rules.check_for_develop_branch(args.directory, name):
195+
develop = True
196+
output = ' development branch "{}" found'.format(name)
197+
print(output)
198+
if not develop:
199+
output = '! no development branch found'
200+
print(output)
201+
129202

130-
# Can add more features to check for in here
131-
# TODO: Could really use refactoring to handle results + output strings
132-
# better, and could pull individual conditionals out into functions
203+
def check_multiple_git_commits(args):
204+
"""Call the check for multiple commits on a branch and print the
205+
result to stdout.
206+
207+
Parameters
208+
----------
209+
args : argparse.Namespace
210+
Arguments input when script is called from the console
211+
212+
Returns
213+
-------
214+
None
215+
"""
216+
multiple_commits = rules.check_for_multiple_commits(args.directory)
217+
if multiple_commits:
218+
output = ' multiple commits on a branch found'
219+
else:
220+
output = '! one or fewer commits on each branch'
221+
print(output)
133222

134223

135224
if __name__ == '__main__':

0 commit comments

Comments
 (0)