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

fix(build): issues/34 app id, path updates #73

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

cdcabrera
Copy link
Member

@cdcabrera cdcabrera commented Aug 5, 2019

What's included

Notes

How to test

Coverage and basic unit test check

  1. update the NPM packages with $ yarn
  2. $ yarn test

Build integration check

  1. update the NPM packages with $ yarn
  2. $ yarn build
  3. once the initial build is on your local you can run some of the integration checks manually$ yarn test:integration-dev
  4. once that's running go ahead and update package.json around line 19 ... change subscriptions to something else and watch the tests catch the name change. This is expected
    "insights": {
     "appname": "subscriptions"
    },
    

Example

...

Updates issue/story

#34

@codecov-io
Copy link

codecov-io commented Aug 5, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #73   +/-   ##
=======================================
  Coverage   88.77%   88.77%           
=======================================
  Files          23       23           
  Lines         303      303           
  Branches       65       65           
=======================================
  Hits          269      269           
  Misses         27       27           
  Partials        7        7

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 a4f7da7...36aaa46. Read the comment docs.

@cdcabrera cdcabrera changed the title WIP fix(build): issues/34 app id, path updates fix(build): issues/34 app id, path updates Aug 6, 2019
@cdcabrera cdcabrera requested a review from priley86 August 6, 2019 17:32
cdcabrera added a commit to cdcabrera/curiosity-frontend that referenced this pull request Aug 6, 2019
* build, dotenv, package, and proxy config name update
* testing, integration checks around naming
const { appname } = packageJson.insights;

const output = execSync(`grep -w "REACT_APP_UI_NAME=${appname}" ${envFile}`);
expect(output.toString().split('\n')).toMatchSnapshot('ui name');
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

cdcabrera added a commit to cdcabrera/curiosity-frontend that referenced this pull request Aug 6, 2019
* build, dotenv, package, and proxy config name update
* testing, integration checks around naming
@cdcabrera
Copy link
Member Author

Almost good to go, little bit of coordination. We're awaiting the platform PR merge, once that happens we can merge this in.

* build, dotenv, package, and proxy config name update
* testing, integration checks around naming
@cdcabrera cdcabrera merged commit d3e7bc2 into RedHatInsights:master Aug 8, 2019
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