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

updates in methods, related to folder paths #149

Open
atherdon opened this issue Jul 14, 2019 · 23 comments
Open

updates in methods, related to folder paths #149

atherdon opened this issue Jul 14, 2019 · 23 comments
Assignees

Comments

@atherdon
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We have a lot of methods here that work with folder paths or file paths.
my concern is - we can easily broke them, by passing anything else that path related.

  1. we need to list here all methods inside of "src" folder files, that somehow related on paths. take your time and ask questions if needed. readme will be helpful as well.
  2. when i'll approve step1 - we'll try to avoid bugs at this methods
  3. we can also use https://www.npmjs.com/package/is-valid-path for our needs here
  4. i think we can create some tests that will show us that we can use anything in arguments on this methods and nothing will go crashing

#13

@atherdon
Copy link
Contributor Author

@Beni03 @elnur004 let's do this task together.

@elnur004
Copy link

@atherdon from which way we need to list methods? may you clarify it a bit, please?

@elnur004
Copy link

i've read the readme but it was inexplicit for me

@elnur004
Copy link

@atherdon may you explain a bit, please?

@atherdon
Copy link
Contributor Author

@svr8 can you advise here?

@svr8
Copy link
Contributor

svr8 commented Jul 19, 2019

Considering the files inside src folder, we need to find all methods that involve path to folder/file. For example: generateArray method and write methods here work with paths.

@elnur004
Copy link

@atherdon i think this is a bit complex task for me, may you change it for me, please?

@atherdon
Copy link
Contributor Author

@svr8 maybe you can help with this task. first we need to have a list of methods that have paths.
then we'll upgrade them in bulk, because we have same "pain points" there

@svr8
Copy link
Contributor

svr8 commented Jul 30, 2019

Here is a list of files:

  • fileSystem.js
    • write()
    • read()
    • save()
    • makeFolder()
  • generateArray.js
    • setupPath()
  • generateFile.js
    • generateFile()
  • generateFiles.js
    • generateFiles()
  • objects.js
    • combine()
    • split()
  • testsMethods.js
    • jsonFileNotEmptyTest()
    • jsonSchemaTest()
  • utils.js
    • checkFilePath()
    • isDirectory()
    • fixPath()
    • getFileInfo()
    • getList()
    • getListContent()
    • readAllFiles()
    • fixPath()

@atherdon
Copy link
Contributor Author

atherdon commented Jul 30, 2019 via email

@atherdon
Copy link
Contributor Author

@IamRaviTejaG check this task and tell me if you think you can improve our methods

@Beni03
Copy link
Contributor

Beni03 commented Sep 5, 2019

@atherdon
Adding these lines of code in each method will improve them.

  1. var isValid = require('is-valid-path');
    //inside each method
  2. if(!isValid(path))
    {
    console.log("path is not valid")
    }
    Example:
    //src/fileSystems/write method
    const write = (path, data) => new Promise((resolve) => {
    if(!isValid(path))
    {
    console.log("path is not valid")
    }

const dataStr = stripSymbols(data);

writeFile(path, dataStr, (err) => {
if (err) {
console.error(err);
resolve(false);
} else {
console.info(${path} file generated successfully!);
resolve(true);
}
});
});

@atherdon
Copy link
Contributor Author

atherdon commented Sep 5, 2019

alright - can you open a pull request with code changes so i can look into it?
don't forget your fork, because we made some configuration changes

@Beni03
Copy link
Contributor

Beni03 commented Sep 5, 2019 via email

@Beni03
Copy link
Contributor

Beni03 commented Sep 10, 2019

image

this is the error I am getting after npm install.
I think because of this error, I could not commit my changes.

@atherdon
Copy link
Contributor Author

atherdon commented Sep 10, 2019 via email

atherdon added a commit that referenced this issue Sep 11, 2019
isValid(path)added in fileSystem/write method (#149)
@atherdon
Copy link
Contributor Author

i have a question. i'm a bit confused. we have a method inside of our src folder. named like isFolderExist.
is it work differently from is-valid-path?
should we use both of them or just use only one?

@Beni03
Copy link
Contributor

Beni03 commented Sep 12, 2019 via email

@atherdon
Copy link
Contributor Author

atherdon commented Sep 12, 2019 via email

@Beni03
Copy link
Contributor

Beni03 commented Sep 15, 2019

error screen

I am geting this error after adding return statement

@atherdon
Copy link
Contributor Author

can you push your changes and open a pull request, so I can checkout it locally and take a look?

@atherdon
Copy link
Contributor Author

atherdon commented Dec 8, 2019

can you take a look at this task and explore was it completed or not @Edebo ?

@Edebo
Copy link
Contributor

Edebo commented Dec 9, 2019

ok

atherdon added a commit that referenced this issue Dec 10, 2019
updates in methods, related to folder paths #149
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

No branches or pull requests

5 participants