search

anitab-org / mentorship-ios

THIS PROJECT IS ARCHIVED. Mentorship System is an application that matches women in tech to mentor each other, on career development, through 1:1 relations during a certain period of time. This is the iOS application of this project.
GNU General Public License v3.0
14 stars 28 forks source link

refactored previous func as computed value #147

Closed R-Cole closed 4 years ago

R-Cole commented 4 years ago

Description

Refactored previous func as a computed value that returns first name of user with any errant whit spaces removed. NavTitle remains set to displayMode: inline.

Fixes # 131

Type of Change:

Delete irrelevant options.

Code/Quality Assurance Only

How Has This Been Tested?

User accounts were created with intentional white spaces before the name. User accounts were created with very long first names.

Checklist:

Delete irrelevant options.

Code/Quality Assurance Only

yugantarjain commented 4 years ago

Hi @R-Cole, thanks for the update. I have a few questions/suggestions, please have a look.

A few additional points:

  1. Can you add a unit test for the new computed property in HomeTests.swift
  2. When you update, no need to create a new PR. Just push you changes to your branch (R-cole:develop) and this PR will get updated automatically.
  3. Also, this build is failing. You must be seeing a 'Details' button at the right side of the row where it says Travis CI failing, these details can be used to fix the problem.
R-Cole commented 4 years ago

Hi @yugantarjain, looks like the chained capitalize is necessary because no field validation for an all caps entry so fixes that. Also I added tests, new to iOS testing but seems to be passing. Not sure about Travis stuff...would very much like to understand though. Also got rid of the nil coalescing.

yugantarjain commented 4 years ago

Hi @yugantarjain, looks like the chained capitalize is necessary because no field validation for an all caps entry so fixes that. Also I added tests, new to iOS testing but seems to be passing. Not sure about Travis stuff...would very much like to understand though. Also got rid of the nil coalescing.

Hi @R-Cole, thanks for the update! Looks like your latest changes haven't been pushed to the PR yet. Also, can you add a screenshot in the description.

R-Cole commented 4 years ago

I tried amending previous PR without success, sorry for new pr.

yugantarjain commented 4 years ago

I tried amending previous PR without success, sorry for new pr.

No problem.