Skip to content

Only precompile test app assets on CI#1392

Merged
josepjaume merged 1 commit intomasterfrom
only_precompile_test_app_on_ci
May 30, 2017
Merged

Only precompile test app assets on CI#1392
josepjaume merged 1 commit intomasterfrom
only_precompile_test_app_on_ci

Conversation

@josepjaume
Copy link
Copy Markdown
Contributor

🎩 What? Why?

Using the test app locally forces us to recompile assets every time something is changed. This deals with it by only precompiling assets using a specific task meant for CI environments.

📌 Related Issues

📋 Subtasks

None

📷 Screenshots (optional)

None

👻 GIF


end

task :generate_static_test_app => :generate_test_app do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use the new Ruby 1.9 hash syntax.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The NEW Ruby 1.9 hash syntax 🖖

@josepjaume josepjaume changed the title Only precompile test app on CI Only precompile test app assets on CI May 23, 2017
@mention-bot
Copy link
Copy Markdown

@josepjaume, thanks for your PR! By analyzing the history of the files in this pull request, we identified @beagleknight and @deivid-rodriguez to be potential reviewers.

@josepjaume josepjaume force-pushed the only_precompile_test_app_on_ci branch 2 times, most recently from 622e41b to 4ccd42d Compare May 23, 2017 07:49
end

def dummy_app_path
dummy_app_path = File.expand_path(File.join(Dir.pwd, "spec", "decidim_dummy_app"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Useless assignment to variable - dummy_app_path.

@josepjaume josepjaume force-pushed the only_precompile_test_app_on_ci branch from 4ccd42d to 9a4911a Compare May 23, 2017 07:51
@beagleknight
Copy link
Copy Markdown
Contributor

This is really cool @josepjaume! There is a problem in the CI tho :/

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

deivid-rodriguez commented May 24, 2017

Yeah, looking forward to this! :)

@josepjaume josepjaume force-pushed the only_precompile_test_app_on_ci branch from 9a4911a to 6d5843d Compare May 30, 2017 11:59
@codecov
Copy link
Copy Markdown

codecov bot commented May 30, 2017

Codecov Report

Merging #1392 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1392   +/-   ##
=======================================
  Coverage   95.42%   95.42%           
=======================================
  Files         428      428           
  Lines        7365     7365           
=======================================
  Hits         7028     7028           
  Misses        337      337

Copy link
Copy Markdown
Contributor

@beagleknight beagleknight left a comment

Choose a reason for hiding this comment

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

Can you add the CI variable to the circle ci config as well? line 6 😄

@josepjaume
Copy link
Copy Markdown
Contributor Author

Already there by default, as in most CI's: https://circleci.com/docs/1.0/environment-variables/

@beagleknight
Copy link
Copy Markdown
Contributor

I wasn't sure about the CircleCi 2.0 because we are using custom containers 😄

@josepjaume
Copy link
Copy Markdown
Contributor Author

@josepjaume josepjaume merged commit 079c79b into master May 30, 2017
@josepjaume josepjaume deleted the only_precompile_test_app_on_ci branch May 30, 2017 12:51
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.

6 participants