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.

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

Update gem & science commands to support auto update #182

Merged

Conversation

mhdsyfq77
Copy link

@mhdsyfq77 mhdsyfq77 commented Nov 6, 2020

Several changes made but main resolves #159

Main change:

  • Gem & science lists now updates whenever a module is added
  • Moved GemCommandStorage methods to GemCommand

Other changes:

  • Added module title to display (i feel like users would like to see module title if not they just see letters and numbers x.x)
  • Shifted gem command messages to commons.core.Messages
  • Separated display of GE commands using the GE pillars (Human Cultures, Asking Questions, etc.) as gem module codes look too congested
  • Resolved test issues that came with changes

# Conflicts:
#	src/main/java/seedu/address/logic/parser/GradPadParser.java
@mhdsyfq77 mhdsyfq77 added this to the v1.4 milestone Nov 6, 2020
@mhdsyfq77 mhdsyfq77 requested a review from a team November 6, 2020 11:11
@mhdsyfq77 mhdsyfq77 self-assigned this Nov 6, 2020
Copy link

@Silvernitro Silvernitro left a comment

Choose a reason for hiding this comment

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

Clean code as always :D just small nits

# Conflicts:
#	src/main/java/seedu/address/logic/commands/RequiredCommand.java
@codecov-io
Copy link

Codecov Report

Merging #182 (c598282) into master (3a7461e) will decrease coverage by 0.03%.
The diff coverage is 87.87%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #182      +/-   ##
============================================
- Coverage     75.67%   75.64%   -0.04%     
  Complexity      586      586              
============================================
  Files            96       96              
  Lines          1768     1774       +6     
  Branches        183      185       +2     
============================================
+ Hits           1338     1342       +4     
  Misses          345      345              
- Partials         85       87       +2     
Impacted Files Coverage Δ Complexity Δ
...main/java/seedu/address/commons/core/Messages.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...java/seedu/address/logic/parser/GradPadParser.java 69.23% <ø> (ø) 14.00 <0.00> (ø)
...in/java/seedu/address/storage/GemCommandPaths.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
.../java/seedu/address/storage/GemCommandStorage.java 100.00% <ø> (+4.25%) 14.00 <0.00> (-5.00) ⬆️
...a/seedu/address/logic/commands/ScienceCommand.java 83.33% <80.00%> (-4.17%) 5.00 <0.00> (ø)
.../java/seedu/address/logic/commands/GemCommand.java 89.13% <87.50%> (-3.98%) 11.00 <5.00> (+5.00) ⬇️
.../seedu/address/logic/commands/RequiredCommand.java 97.39% <100.00%> (+0.04%) 36.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a7461e...c598282. Read the comment docs.

@yan-soon yan-soon merged commit d2b96a4 into AY2021S1-CS2103T-T09-1:master Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PE-D] Modules appearing under gem modules even though they have already been taken
5 participants