Skip to content

Commit

Permalink
Reviewed and improved Issues throughout the code
Browse files Browse the repository at this point in the history 
 - Removed Issues that have already been resolved.
 - Removed Issues that are no longer relevant, or that it was decided
   not to address any time soon.
 - Improved properties for various Issues.
 - Removed 'release' label, and added '1.0.0-beta1' where appropriate.
  • Loading branch information
@krystalcode
krystalcode committed Aug 4, 2016
1 parent 037c21e commit 7c0d028
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 157 deletions.
68 changes: 4 additions & 64 deletions app/Main.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,88 +31,28 @@ main = do
-- Issues.

{-
@Issue(
"Rename the program to unhack-engine"
type="task"
priority="low"
)
@Issue(
"Develop and support an improved annotation style"
type="feature"
priority="high"
labels="release"
labels="1.0.0-beta1"
)
@Issue(
"Move these issues to the unhack.yaml file"
type="task"
priority="low"
)
@Issue(
"Add error handling in command line"
type="bug"
priority="low"
labels="ux"
)
@Issue(
"Colorise command line output"
type="improvement"
priority="low"
labels="ux"
)
@Issue(
"Add configuration options for defining colors per property"
type="feature"
priority="low"
labels="ux"
)
@Issue(
"Move command line handling into a separate module"
type="task"
priority="low"
labels="structure"
)
@Issue(
"Add verbosity option and print progress statements"
type="feature"
priority="low"
labels="ux"
)
@Issue(
"Add logging"
type="feature"
priority="normal"
labels="log management"
)
@Issue(
"Add filtering options"
"Remove screen printing and add proper logging and error management
throughout the application"
type="feature"
priority="normal"
)
@Issue(
"Add statistics command e.g. available labels, number of issues
filtered by type or priority etc."
type="feature"
priority="low"
labels="editor integration"
)
@Issue(
"Add statistics command e.g. available labels, number of issues
filtered by type or priority etc."
type="feature"
priority="low"
labels="editor integration"
)
@Issue(
"Convert modes to process based [issues, statistics] and add source
[git, filesystem] and output [shell, json, storage] options"
type="improvement"
priority="normal"
labels="modularity"
)
@Issue(
"Remove all screen printing in production environment"
type="improvement"
priority="low"
labels="performance"
labels="1.0.0-beta1, log management"
)
-}
8 changes: 1 addition & 7 deletions src/Unhack/Cmd/PubSub.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,7 @@ runPubSub cmd = do
"Log an error message for unrecognised channels"
type="bug"
priority="low"
labels="log management"
)
@Issue(
"Is there really any case where we would received messages on unrecognised channels?"
type="investigation"
priority="low"
labels="clean up"
labels="error management, log management"
)
-}
False -> putStrLn $ "The Redis PubSub channel '" ++ (BS.unpack channel) ++ "' is not recognised."
Expand Down
17 changes: 0 additions & 17 deletions src/Unhack/Git/Fetch.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@ import qualified Unhack.Git.Location as UGL (base)

-- Public API.

{-
@Issue(
"Add a function that does a clone or fetch depending on whether the
repository is already cloned"
type="improvement"
priority="low"
)
-}

-- Clone a git repository.
clone :: T.Text -> T.Text -> T.Text -> IO (T.Text)
clone vendor owner repository = do
Expand Down Expand Up @@ -68,14 +59,6 @@ fetch directory branches = do

-- Functions/types for internal use.

{-
@Issue(
"Successful fetch outputs the result as an empty string"
type="bug"
priority="low"
)
-}

-- Execute a 'git fetch' command with the given options.
fetch' :: FilePath -> T.Text -> IO (T.Text)
fetch' directory options = lazyProcess command directory
Expand Down
7 changes: 0 additions & 7 deletions src/Unhack/Issue.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ import Unhack.Data.IssueProperties (IssueProperties)
data Issue = Issue
{ commit :: EmIssueCommit
, createdAt :: UTCTime
{-
@Issue(
"Support path hierarchy in file property",
type="improvement",
priority="high"
)
-}
, file :: T.Text
, properties :: IssueProperties
, repository :: EmbeddedRepository
Expand Down
15 changes: 4 additions & 11 deletions src/Unhack/Parser.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ import Unhack.Util (csvToList)
{-
@Issue(
"Use Text instead of String everywhere in the Parser",
type="task",
priority="normal"
type="improvement",
priority="low"
labels="performance"
)
-}

Expand Down Expand Up @@ -71,14 +72,6 @@ extractMultiValueProperty issue property

where maybeCSVValue = extractProperty issue property

{-
@Issue(
"Allow for different regexps per property e.g. do not allow special
characters in type, priority and labels etc.",
type="bug",
priority="normal"
)
-}
extractProperty :: String -> String -> Maybe T.Text
extractProperty issue property = if value == "" then Nothing else Just (T.pack value)
where pattern = property ++ "=\"([^\"]+)\""
Expand All @@ -89,7 +82,7 @@ extractTitle issue = if T.null validTitle then T.pack issue else validTitle
where maybeTitleWithKey = extractProperty issue "title"
{-
@Issue(
"Reuse extractProperty when it accepts a pattern as an argument",
"Reuse extractProperty by passing pattern as an argument",
type="improvement",
priority="low",
labels="refactoring"
Expand Down
3 changes: 3 additions & 0 deletions src/Unhack/Process.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import qualified Data.Text as T (pack, unpack, Text)
"Implement error handling for the lazyProcess function as well"
type="bug"
priority="normal"
label="error management"
)
-}

Expand Down Expand Up @@ -58,6 +59,7 @@ strictProcess command directory = handle (\e -> handleException e) $ do
"Capture and return the exception message as well"
type="improvement"
priority="low"
labels="error management"
)
-}
if exitCode == ExitSuccess
Expand All @@ -73,6 +75,7 @@ strictProcess command directory = handle (\e -> handleException e) $ do
"Differentiate between IOException and other exceptions"
type="improvement"
priority="low"
labels="error management"
)
-}
where handleException (SomeException e) = return $ Left (ExitFailure 0)
Expand Down
14 changes: 0 additions & 14 deletions src/Unhack/Pubsub/Dispatcher.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ import qualified Unhack.Storage.ElasticSearch.Config as USEC (indexSettingsFromC

-- Dispatch an incoming pubsub message to the appropriate handling function.
{-
@Issue(
"Log message when the action is completed instead of printing on screen"
type="improvement"
priority="low"
labels="log management"
)
@Issue(
"Support json-encoded pubsub messages"
type="bug"
Expand All @@ -47,14 +41,6 @@ dispatch config message = do
let repositoryId = T.pack (BS.unpack $ messageParts !! 1)
print $ "Dispatching message of type '" ++ action ++ "'"
UPR.clone config (USEC.indexSettingsFromConfig "repository" config) repositoryId
{-
@Issue(
"Execute the remaining tasks according to the flow"
type="bug"
priority="normal"
labels="release"
)
-}
print $ "Action of type '" ++ action ++ "' performed"

-- Request to analyse all commits for the active branches.
Expand Down
63 changes: 29 additions & 34 deletions src/Unhack/Pubsub/Repository.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,6 @@ import qualified Unhack.Storage.ElasticSearch.Operations as USEO (bulkInde
-}

-- Pubsub task for cloning a repository.
{-
@Issue(
"Refactor fetching and extracting a document into a separate function"
type="improvement"
priority="normal"
labels="refactoring"
)
-}
clone :: USEC.StorageConfig -> USEC.StorageIndexSettings -> T.Text -> IO ()
clone config indexSettings repositoryId = do

Expand Down Expand Up @@ -109,6 +101,7 @@ clone config indexSettings repositoryId = do
user"
type="bug"
priority="low"
labels="error management"
)
-}
cloneResult <- UGF.clone vendor owner repositoryName
Expand Down Expand Up @@ -185,21 +178,20 @@ analyseAll storageConfig indexSettings repositoryId = do
different per repository"
type="improvement"
priority="low"
labels="performance, release"
labels="performance"
)
@Issue(
"Log number of commits analysed by the 'analyse_recent' task in order to find a safe number of
recent commits per repository"
type="improvement"
priority="low"
labels="analytics, release"
labels="analytics"
)
@Issue(
"Consider detecting commits not processed via cron job, in case they escape the
'analyse_recent' task"
type="bug"
priority="low"
labels="release"
)
-}
mCommitsBranchNamesWithHashes <- UGCom.list directory branchesToAnalyseNames
Expand Down Expand Up @@ -299,7 +291,7 @@ analyseAll storageConfig indexSettings repositoryId = do
{- @Issue( "Handle errors"
type="bug"
priority="low"
labels="error management, log management" )
labels="error management" )
@Issue( "Investigate a solution for better distribution of worker resources between repositories
that run on the same engine instance"
type="bug"
Expand All @@ -309,15 +301,6 @@ analyseAll storageConfig indexSettings repositoryId = do

-- Send a pubsub message to update the head commits for all active branches, and for the repository
-- itself as well.
{-
@Issue(
"The updated records for the head commits are not available to search when this task is
executed"
type="bug"
priority="normal"
labels="release"
)
-}
pubsubResponse <- UPP.publish [(repositoryId, T.intercalate ":" ["repositories_update_heads", repositoryId])]

return mempty
Expand Down Expand Up @@ -396,14 +379,14 @@ analyseCommits storageConfig indexSettings repositoryId commitsIds = do
)
@Issue(
"Support constructing a build message together with the build status"
type="bug"
priority="high"
type="improvement"
priority="low"
)
@Issue(
"Support storing log messages that would be used to display in the web UI any errors
that occurred during the build"
type="feature"
priority="normal"
priority="low"
)
-}
let flatIssuesProperties = map (\ (commit, filesIssuesProperties)
Expand Down Expand Up @@ -445,7 +428,7 @@ analyseCommits storageConfig indexSettings repositoryId commitsIds = do
where properly stored"
type="bug"
priority="normal"
labels="log management"
labels="error management"
)
-}

Expand Down Expand Up @@ -483,6 +466,15 @@ updateHeads storageConfig indexSettings repositoryId = do
-- If a head commit was analysed immediately before running this task, it
-- may not be immediately available to search due to Elastic Search's near
-- real time feature. Delay 1s to make sure that the commit will be found.
{-
@Issue(
"Consider passing on calling this task with the commits as arguments, or
postponing execution for 1s and proceed with processing other tasks"
type="improvement"
priority="low"
labels="performance"
)
-}
threadDelay 1000

-- Get the repository record.
Expand All @@ -504,7 +496,7 @@ updateHeads storageConfig indexSettings repositoryId = do
-- (absence of 'master' branch) an analysis wouldn't be triggered in the first place.
{-
@Issue(
"Log a message instead of throwing an error"
"Log a message instead of throwing an error when there are no active branches"
type="bug"
priority="low"
labels="error management, log management"
Expand All @@ -529,14 +521,8 @@ updateHeads storageConfig indexSettings repositoryId = do
-- that will be embedded in the branch/repository records.
{-
@Issue(
"Restrict query by repository id filtering"
type="improvement"
priority="low"
labels="performance"
)
@Issue(
"Log an error if a record is not found for the git commit
instead of letting the program fail"
"Log an error if a record is not found for one or more git
commits instead of letting the program fail"
type="bug"
priority="low"
labels="error management"
Expand All @@ -556,6 +542,15 @@ updateHeads storageConfig indexSettings repositoryId = do
let maybeCommitsWithIds = map (\hit -> (hitDocId hit, hitSource hit)) resultHits
let justCommitsWithIds = filter (\(commitId, commit) -> isJust commit) maybeCommitsWithIds
let commitsWithIds = map (\(commitId, commit) -> (commitId, fromJust commit)) justCommitsWithIds

{-
@Issue(
"Restrict commits to repository when making the ES query instead of filtering the results"
type="improvement"
priority="low"
labels="performance"
)
-}
let filteredCommitsWithIds = filter (\(commitId, commit) -> repositoryId == UDC.repositoryId commit) commitsWithIds

let emCommits = UDEC.fromCommits filteredCommitsWithIds
Expand Down
Loading

0 comments on commit 7c0d028

Please sign in to comment.