Updated Assessment #29
Conversation
assessment.md
Outdated
| * I = Pushed to master or non-descriptive commits | ||
| * A = Pushed to correct repository with descriptive commits | ||
| * N = GitHub flow model unused or no commits to the Github repository | ||
| * I = Pushed to master or non-descriptive commits to the Github repository |
There was a problem hiding this comment.
These two should end with made to the GitHub repository
| * A = Pushed to correct repository with descriptive commits | ||
| * N = GitHub flow model unused or no commits to the Github repository | ||
| * I = Pushed to master or non-descriptive commits to the Github repository | ||
| * A = Pushed to correct repository with descriptive commits to the Github |
There was a problem hiding this comment.
This repeats the word repository unnecessarily, and should be reworded. There shouldn't be a need to append to the repository on the end of each of these sentences.
There was a problem hiding this comment.
Does it look better now?
There was a problem hiding this comment.
The line that was the problem is line 124 and 125 -- however this change is good as well!
| * G = Use of a branch/fork demonstrated appropriately | ||
| (e.g having a feature branch) along with descriptive commits | ||
| * E = Use of multiple useful branches/forks demonstrated appropriately | ||
| containing multiple coherent and descriptive commits | ||
| containing multiple consistent and descriptive commits |
There was a problem hiding this comment.
Coherent and consistent do not mean the same thing -> is the change in meaning acceptable?
I would argue that coherent commits are incredibly necessary.
There was a problem hiding this comment.
I no longer have the paper with me, but if I remember correctly this change was suggested by @gkapfham.
| * A = Communicating changes with the team and reconciling differences | ||
| * G = Merging branches after extensive communication | ||
| * E = Merging branches with well-documented, detailed code and | ||
| extensive communication | ||
| extensive team communication |
There was a problem hiding this comment.
Communication is not necessarily in teams here... is there some justification for this change?
|
|
||
| ### Continuous Integration with Travis | ||
|
|
||
| * Setup and configure Travis CI | ||
| * N = Fails to perform any setup and configuration of Travis CI | ||
| * I = Sets up and configures Travis CI which performs no checks | ||
| * A = Sets up and configures Travis CI without performing checks on less | ||
| * A = Sets up and configures Travis CI that performs checks on less |
There was a problem hiding this comment.
Wait.... this inverts the meaning of the sentence, and seems that setting up checks on less relevant project deliverables is not as desirable? This is not clear.
|
|
||
| ### Foundations of Software Engineering | ||
|
|
||
| * Requirements engineering | ||
| * N = No effort to document, define, or maintain requirements present | ||
| * N = No effort to document, define, or maintain requirements present is | ||
| evident |
There was a problem hiding this comment.
is evident is redundant, present implies the same idea.
|
@finneyj2 @Lancasterwu can both of you try to address the issues raised here so that this can be merged soon? |
|
@Michionlion I no longer have the paper with me, but if I remember correctly, most of the changes that I implemented were suggested by @gkapfham. Possibly all of them that are being questioned in this pull request. I think I made earlier errors and typos that I addressed earlier. |
Made advised corrections to the categories Project Management with GitHub, Continuous Integration with Travis, and Foundations of Software Engineering. Mostly made grammar corrections, but one sentence had to be partially rewritten.