Skip to content

Road to 1.0 - Refactor project structure#142

Merged
f-meloni merged 9 commits intomasterfrom
refactor-project-structure
Nov 30, 2020
Merged

Road to 1.0 - Refactor project structure#142
f-meloni merged 9 commits intomasterfrom
refactor-project-structure

Conversation

@gianluz
Copy link
Member

@gianluz gianluz commented Nov 25, 2020

  • Refactored danger-kotlin-library in a more meaningful package structure.
  • Extracted some components in different classes avoiding god kotlin files
  • Changed visibility for the DSL model to internal
  • Kdocs


@Serializable
data class DSL(
internal data class DSL(
Copy link
Member Author

Choose a reason for hiding this comment

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

the DSL object shouldn't be visible from the Danger File, DangerDSL yes

Copy link
Member

Choose a reason for hiding this comment

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

I always forget that in kotlin default is public, because on Swift the default is internal 😅


@Serializable
data class Meta(
internal data class Meta(
Copy link
Member Author

Choose a reason for hiding this comment

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

Meta as well shouldn't be visible from the Danger File as well, changed to internal

}

class ShellExecutorImpl : ShellExecutor {
internal class ShellExecutorImpl : ShellExecutor {
Copy link
Member Author

Choose a reason for hiding this comment

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

our internal implementation for the shell executor shouldn't be visible from the danger file, we can access by interface

import systems.danger.kotlin.models.git.FilePath
import java.io.File

internal object JsonParser {
Copy link
Member Author

Choose a reason for hiding this comment

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

Json parser extracted

import systems.danger.kotlin.sdk.Violation
import java.io.File

internal class MainDangerRunner(jsonInputFilePath: FilePath, jsonOutputPath: FilePath) : DangerContext {
Copy link
Member Author

@gianluz gianluz Nov 25, 2020

Choose a reason for hiding this comment

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

Danger runner is now composed by 3 files: MainDangerRunner that contains the private implementation, MainPlugins that contains the helpers for registering and using Danger plugins and MainScript that contains all the API functions like message warn and initialisation val danger=Danger(args) or danger(args) {...} block.

Extensions onGit onGitHub .. moved to KtxDangerDSL.kt

@danger-public
Copy link

danger-public commented Nov 25, 2020

Warnings
⚠️ Big PR, try to keep changes smaller if you can

Generated by 🚫 Danger Kotlin against 768b67f

@gianluz gianluz requested a review from f-meloni November 27, 2020 10:43
@github-actions
Copy link

github-actions bot commented Nov 28, 2020

Warnings
⚠️ Big PR, try to keep changes smaller if you can

Generated by 🚫 Danger Kotlin against 768b67f

Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

A lot of good stuff!

@@ -0,0 +1,5 @@
package systems.danger.kotlin
Copy link
Member

Choose a reason for hiding this comment

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

Do we want something for the future? Or we can add it when is needed?
Looks a little bit a premature optimisation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Up to us! We can delete the empty stuff for now, but i like the idea that we have already a place where to put new extensions if needed


@Serializable
data class DSL(
internal data class DSL(
Copy link
Member

Choose a reason for hiding this comment

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

I always forget that in kotlin default is public, because on Swift the default is internal 😅

* - [Sdk.API_VERSION]
* - [Sdk.VERSION_NAME]
*
* @constructor Create empty Sdk
Copy link
Member

Choose a reason for hiding this comment

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

Does anyone need to create an SDK object? Is there a way to make this impossible?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is a singleton with two fields, in this case works like a static class with const fields 😄 this is how works.
using object in kotlin is also a way to group under the same context static fields

…inPlugins.kt

Co-authored-by: Franco Meloni <franco.meloni91@gmail.com>
@f-meloni f-meloni merged commit 1817907 into master Nov 30, 2020
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.

3 participants