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

[WIP] 117 generator #144

Closed
wants to merge 13 commits into from
Closed

[WIP] 117 generator #144

wants to merge 13 commits into from

Conversation

Hajto
Copy link
Collaborator

@Hajto Hajto commented Jul 7, 2017

One step generator based on the one that phoenix have.

[WIP] - Please don't touch since a lot of things will chagne

/Edit wende: Closes #117

@Hajto Hajto self-assigned this Jul 7, 2017
@Hajto Hajto requested a review from wende July 7, 2017 18:24
@wende wende changed the title [HEAVY WIP] 117 generator [WIP] 117 generator Jul 22, 2017
@wende
Copy link
Owner

wende commented Jul 22, 2017

I'd appreciate some tests checking if generation works properly

# variables.
/config/prod.secret.exs

# -*- mode: gitignore; -*-
Copy link
Owner

Choose a reason for hiding this comment

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

Something went wrong here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What exactly?

Copy link
Owner

Choose a reason for hiding this comment

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

As far as I know Emacs mode should be at the first line to work properly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:/ As far as I can remember its fresh elixir generated gitignor

Copy link
Owner

@wende wende left a comment

Choose a reason for hiding this comment

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

Looks fine but it'd make a use of some cleaning. Too many commented out lines and generally test-code

"exposed-modules": [],
"dependencies": {
"elm-lang/core": "5.1.1 <= v < 6.0.0",
"wende/elmchemy-core": "1.0.0 <= v < 2.0.0"
Copy link
Owner

@wende wende Sep 1, 2017

Choose a reason for hiding this comment

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

the correct one is "wende/elchemy-core": "0.0.0 <= v < 1.0.0"
But probably it'd be better to copy it entirely from the current /templates/elm-package.json

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,5 @@
defmodule CharacterTest do
use ExUnit.Case
use Elmchemy
Copy link
Owner

Choose a reason for hiding this comment

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

A good template for testing would be

defmodule <%= app_module %>Test do
   use ExUnit.Case
   use Elchemy

   doctest <%= app_module %>
   typetest <%= app_module %>
   
    test do
      assert <%= app_module %>.hello == "world"
    end
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@baransu
Copy link
Collaborator

baransu commented Oct 7, 2017

What the status of this PR. I would like to take it and lead it into merge.

/ cc @wende @Hajto

@Hajto
Copy link
Collaborator Author

Hajto commented Oct 7, 2017

@baransu Go ahead, I am dead out of time to work on it. I will be gratefull!

@wende
Copy link
Owner

wende commented Sep 26, 2018

Closing the PR till somebody picks it up

@wende wende closed this Sep 26, 2018
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