Merged
Conversation
17416dd to
e44bcd1
Compare
Check that they have uppercase names longer than five characters Add unit tests for checks.
- Update tests to use Line objects - Use new style class for Line
e44bcd1 to
5f1c861
Compare
ned21
previously requested changes
Dec 9, 2018
OK I guess the API should match the usual use, not the unit tests.
ned21
approved these changes
Nov 13, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Modelling lines (and eventually other parts of the checks) as objects makes it much simpler to keep related information together as it moves around and opens up the possibility for decoupling the analysis and presentation layers.
This is part one of five major re-factoring steps that were undertaken at RAL, it has taken some time to clean-up the changes into something that can be reviewed in a sane way.
Requires #219.