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

Expand variable and type parameter name #77

Merged

Conversation

kmhjs
Copy link
Contributor

@kmhjs kmhjs commented Jul 9, 2017

Hi. I would like to submit this PR for following features.

Note that, this PR contains an update written in # Note section. (Have effects to current codes)
If possible, I want to discuss about this feature can be included or not.

Thanks.


Abstract

This PR changes following things

  • Expand type parameter name
    • Set type parameter name as ${Target}Type
    • Set type parameter name (declared with associatedtype ) as ${Target}
  • Expand variable name
    • Expand few variable names
    • Make variable name more clear

Note

!!! Modifications in this PR breaks following parts.

  • ContextType -> Context

kmhjs added 3 commits July 8, 2017 15:25
Abstract:

- Set type parameter name as "${Target}Type"
- Set type parameter name (declared with `associatedtype` ) as "${Target}"

Note:

- This commit will break following thing.
  - `ContextType` declared in `Scene` protocol.
    - `ContextType` must be replaced with `Context` .
Abstract:

- Expand few variable names
- Make variable name more clear
Abstract:

- Update `ContextType` to `Context`
@codecov-io
Copy link

codecov-io commented Jul 9, 2017

Codecov Report

Merging #77 into versions/0.4.0 will not change coverage.
The diff coverage is 86.2%.

Impacted file tree graph

@@               Coverage Diff               @@
##           versions/0.4.0      #77   +/-   ##
===============================================
  Coverage           93.49%   93.49%           
===============================================
  Files                   7        7           
  Lines                 123      123           
===============================================
  Hits                  115      115           
  Misses                  8        8
Impacted Files Coverage Δ
KabuKit/Classes/core/Swift/Scene.swift 0% <0%> (ø) ⬆️
KabuKit/Classes/core/Swift/SceneOperation.swift 94.11% <100%> (ø) ⬆️
KabuKit/Classes/core/Swift/SceneSequence.swift 100% <100%> (ø) ⬆️
KabuKit/Classes/core/Swift/Screen.swift 85.71% <50%> (ø) ⬆️
KabuKit/Classes/core/Swift/Scenario.swift 94% <93.75%> (ø) ⬆️

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 8f5dab0...fcfa5ea. Read the comment docs.


fileprivate var dic = [String : Transitioning]()
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@crexista
Copy link
Owner

Please pass PR Builder check 🐱

Abstract:

- Adds test cases for SceneSequenceTest, SceneTest, ScreenTest
@kmhjs
Copy link
Contributor Author

kmhjs commented Jul 20, 2017

#77 (comment)

Sorry for late.

I added few test cases for SceneSequenceTest, SceneTest, ScreenTest .

Copy link
Owner

@crexista crexista left a comment

Choose a reason for hiding this comment

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

Goooooooood!!!!!!!!
LGTM

@crexista crexista merged commit e4a6146 into crexista:versions/0.4.0 Jul 21, 2017
@kmhjs kmhjs deleted the feature/20170708_expand_variable_name branch July 21, 2017 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants