fix: remove duplicate import and no relative import #4015

maze-runnar · 2020-02-06T12:21:58Z

Fixes #3940
fixing es-lint warning for duplicate import and relative parent imports

Checklist

I have read the Contribution & Best practices Guide.
My branch is up-to-date with the Upstream development branch.
The acceptance, integration, unit tests and linter pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

kushthedude · 2020-02-07T11:05:25Z

Isnt @snitin315 working on it ?

snitin315 · 2020-02-07T11:10:23Z

@maze-runnar you should have notified before making PR as per the guidelines. Its ok for this issue . Hope you won't repeat this again in future.

maze-runnar · 2020-02-07T11:42:15Z

@maze-runnar you should have notified before making PR as per the guidelines. Its ok for this issue . Hope you won't repeat this again in future.

Sorry , i just missed that for the first and last time, i will definitily take care of this in future.

iamareebjamal · 2020-02-15T08:40:56Z

Status?

maze-runnar · 2020-02-15T10:23:27Z

Status?

completed...i think 😐 . is there anything remaining?

snitin315 · 2020-02-15T10:27:16Z

tests/test-helper.js

 import QUnit from 'qunit';
-import config from '../config/environment';
+import config from 'open-event-frontend/config/environment';


Are you sure this is correct way?

@snitin315 , can you please pull these changes to test. it's working as far i tested for everyone's concern if you once please check it.

Are you sure this is correct way?

I think it is, other files of project used this type of import.

I'm not sure , but I think somewhere I read that. In ember.js the project-name/ refers to the app folder by default. Please check if the statement is correct .

@snitin315 app.js uses same kind of import -

import Application from '@ember/application'; import Resolver from './resolver'; import loadInitializers from 'ember-load-initializers'; import config from 'open-event-frontend/config/environment'; const App = Application.extend({ modulePrefix : config.modulePrefix, podModulePrefix : config.podModulePrefix, Resolver }); loadInitializers(App, config.modulePrefix); export default App;

Check This

import Application from '@ember/application';
import Resolver from './resolver';
import loadInitializers from 'ember-load-initializers';
import config from 'open-event-frontend/config/environment';
const App = Application.extend({
modulePrefix : config.modulePrefix,
podModulePrefix : config.podModulePrefix,
Resolver
});
loadInitializers(App, config.modulePrefix);
export default App;

when life is unfair 😕 :

kushthedude · 2020-02-15T10:29:49Z

Why I see failing checks if the PR is complete ?

…

On Sat, 15 Feb, 2020, 15:57 Nitin Kumar, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/test-helper.js <#4015 (comment)> : > import QUnit from 'qunit'; -import config from '../config/environment'; +import config from 'open-event-frontend/config/environment'; Are you sure this is correct way? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4015?email_source=notifications&email_token=AKQMTLQYXWZNCJM2F2LOJWTRC67Q7A5CNFSM4KQ36JMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCVVRFWA#pullrequestreview-359338712>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKQMTLX2FE6IMVDMRTBXRRDRC67Q7ANCNFSM4KQ36JMA> .

iamareebjamal · 2020-02-15T10:29:50Z

completed...i think 😐 . is there anything remaining?

Considering that the build is failing, yes

kushthedude · 2020-02-15T10:31:40Z

Does this even work on your local? Is site running after this changes ?

…

On Sat, 15 Feb, 2020, 15:59 Areeb Jamal, ***@***.***> wrote: completed...i think neutral_face . is there anything remaining? Considering that the build is failing, yes — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4015?email_source=notifications&email_token=AKQMTLWIZKX2Q45HDGY3ZRTRC67Z7A5CNFSM4KQ36JMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL3HDZY#issuecomment-586576359>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKQMTLQVW7HKFOMIL4TJOUTRC67Z7ANCNFSM4KQ36JMA> .

maze-runnar · 2020-02-15T10:36:36Z

Does this even work on your local? Is site running after this changes ?

yes, it's running with same code.

iamareebjamal · 2020-02-15T10:47:04Z

The only issue is in the test loader

maze-runnar · 2020-02-15T10:54:57Z

The only issue is in the test loader

How to resolve this?

iamareebjamal · 2020-02-15T10:55:25Z

By fixing it

codecov · 2020-02-15T11:16:21Z

Codecov Report

Merging #4015 into development will increase coverage by 0.07%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           development    #4015      +/-   ##
===============================================
+ Coverage        21.73%   21.80%   +0.07%     
===============================================
  Files              452      451       -1     
  Lines             4726     4724       -2     
===============================================
+ Hits              1027     1030       +3     
+ Misses            3699     3694       -5

Impacted Files	Coverage Δ
app/app.js	`100.00% <0.00%> (ø)`	⬆️
app/helpers/currency-symbol.js	`100.00% <0.00%> (ø)`	⬆️
app/services/raven.js
app/services/auth-manager.js	`53.44% <0.00%> (+1.66%)`	⬆️

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 0456445...327a6ea. Read the comment docs.

maze-runnar · 2020-02-15T11:48:25Z

By fixing it

@iamareebjamal , Done!

kushthedude · 2020-02-15T12:21:18Z

Why there another app.js was added ?

maze-runnar · 2020-02-15T12:24:32Z

Why there another app.js was added ?

https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-relative-parent-imports.md
first approach from the given link.

@kushthedude So this link is telling wrong appraoch about how to remove relative imports.

iamareebjamal · 2020-02-15T12:27:06Z

Didn't see that. Reverting

This reverts commit 3ce9b50.

maze-runnar added 4 commits February 6, 2020 17:17

remove duplicate import

cccb573

remove dulicate import

8c1991a

remove relative import

98ec6d6

remove relative parent import

c636e85

auto-label bot added the fix label Feb 6, 2020

snitin315 reviewed Feb 15, 2020

View reviewed changes

Merge branch 'development' into patch-10

25856e7

Update test-helper.js

832796b

maze-runnar added 2 commits February 15, 2020 17:00

adding app.js to same folder to avoid relative parent import

9dae189

remove relative parent import

327a6ea

maze-runnar requested a review from iamareebjamal February 15, 2020 11:47

iamareebjamal merged commit 3ce9b50 into fossasia:development Feb 15, 2020

maze-runnar deleted the patch-10 branch February 15, 2020 12:14

iamareebjamal added a commit that referenced this pull request Feb 15, 2020

Revert "fix: remove duplicate import and no relative import (#4015)"

7792a82

This reverts commit 3ce9b50.

iamareebjamal mentioned this pull request Feb 15, 2020

Revert "fix: remove duplicate import and no relative import" #4094

Merged

maze-runnar mentioned this pull request Feb 15, 2020

chore: fix relative import #4096

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove duplicate import and no relative import #4015

fix: remove duplicate import and no relative import #4015

maze-runnar commented Feb 6, 2020

kushthedude commented Feb 7, 2020

snitin315 commented Feb 7, 2020

maze-runnar commented Feb 7, 2020

iamareebjamal commented Feb 15, 2020

maze-runnar commented Feb 15, 2020

snitin315 Feb 15, 2020

maze-runnar Feb 15, 2020

maze-runnar Feb 15, 2020

snitin315 Feb 15, 2020

maze-runnar Feb 15, 2020

maze-runnar Feb 15, 2020 •

edited

Loading

kushthedude commented Feb 15, 2020 via email

iamareebjamal commented Feb 15, 2020 •

edited

Loading

kushthedude commented Feb 15, 2020 via email

maze-runnar commented Feb 15, 2020

iamareebjamal commented Feb 15, 2020

maze-runnar commented Feb 15, 2020

iamareebjamal commented Feb 15, 2020

codecov bot commented Feb 15, 2020 •

edited

Loading

maze-runnar commented Feb 15, 2020

kushthedude commented Feb 15, 2020

maze-runnar commented Feb 15, 2020 •

edited

Loading

iamareebjamal commented Feb 15, 2020

fix: remove duplicate import and no relative import #4015

fix: remove duplicate import and no relative import #4015

Conversation

maze-runnar commented Feb 6, 2020

Checklist

kushthedude commented Feb 7, 2020

snitin315 commented Feb 7, 2020

maze-runnar commented Feb 7, 2020

iamareebjamal commented Feb 15, 2020

maze-runnar commented Feb 15, 2020

snitin315 Feb 15, 2020

Choose a reason for hiding this comment

maze-runnar Feb 15, 2020

Choose a reason for hiding this comment

maze-runnar Feb 15, 2020

Choose a reason for hiding this comment

snitin315 Feb 15, 2020

Choose a reason for hiding this comment

maze-runnar Feb 15, 2020

Choose a reason for hiding this comment

maze-runnar Feb 15, 2020 • edited Loading

Choose a reason for hiding this comment

kushthedude commented Feb 15, 2020 via email

iamareebjamal commented Feb 15, 2020 • edited Loading

kushthedude commented Feb 15, 2020 via email

maze-runnar commented Feb 15, 2020

iamareebjamal commented Feb 15, 2020

maze-runnar commented Feb 15, 2020

iamareebjamal commented Feb 15, 2020

codecov bot commented Feb 15, 2020 • edited Loading

Codecov Report

maze-runnar commented Feb 15, 2020

kushthedude commented Feb 15, 2020

maze-runnar commented Feb 15, 2020 • edited Loading

iamareebjamal commented Feb 15, 2020

maze-runnar Feb 15, 2020 •

edited

Loading

iamareebjamal commented Feb 15, 2020 •

edited

Loading

codecov bot commented Feb 15, 2020 •

edited

Loading

maze-runnar commented Feb 15, 2020 •

edited

Loading