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

Export external/mds #472

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Export external/mds #472

wants to merge 6 commits into from

Conversation

bennapp
Copy link
Contributor

@bennapp bennapp commented Jul 28, 2022

Motivation

This was ran from the rake mds:export helper in bigmaven (that script converts file paths that will only work in bigmaven to relative file paths which are needed for standalone external mds). This also includes many lint fixes that were required from bigmaven.

We also had to make some small configuration changes like changing a linter to get the results of the export green.

Acceptance Criteria

PR upkeep checklist

  • Change log entry
  • Label(s)
  • Assignee(s)
  • Deployment URL: https://mavenlink.github.io/design-system/$BRANCH/
  • (When ready for review) Reviewer(s)
  • Green Bigmaven CI check
  • DevQA on Bigmaven staging (e.g. does the work need to be imported in the internal design system?)

@bennapp bennapp requested review from juanca and JustinRyanH July 28, 2022 17:44
@bennapp bennapp self-assigned this Jul 28, 2022
'import/extensions': ['error', 'always', {
ignorePackages: true,
}],
'import/extensions': 'off',
Copy link
Contributor

Choose a reason for hiding this comment

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

We might still want this. One of the common "slowness" of webpack is resolving file paths to actual files. If it has to guess which extension it is, it slows it down a bit at a time.


it('"serves" on http://localhost', async () => {
const body = await (await fetch('http://localhost/api/v1/hello')).text();

expect(body).toEqual('Hello from MSW!');
});

it('errors on unhandled requests', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this failing? I would assume we want this behavior.

@@ -1,5 +1,5 @@
/* Downloaded XML file from https://www.currency-iso.org/en/home/tables/table-a1.html and converted to javascript */
export default {
const currencies = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a specific reason for this change? I'm curious why this was necessary.

expect(await findSelectedOption('test label', 'Option 7')).toBeInTheDocument();
expect(await findSelectedOption('test label', 'USD')).toBeInTheDocument();
expect(await findSelectedOption('test label', 'Option 9')).toBeInTheDocument();
expect(await findSelectedOption('Foo')).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT, findSelectedOption takes 2 arguments: findSelectedOption(fieldLabel, optionLabel)

@@ -73,7 +71,7 @@ describe('<MultiAutocompleter>', () => {

await openOptions('test label');

expect(await findAvailableOption('test label', 'Test Option')).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT, findAvailableOption(fieldLabel, optionLabel).

@JustinRyanH
Copy link
Contributor

@bennapp Do we need this still?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants