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

docs: explicit call out for when process.cwd is set #1959

Closed
wants to merge 2 commits into from
Closed

Conversation

rhettjay
Copy link
Member

@rhettjay rhettjay commented Dec 15, 2023

Add clarity to docs to resolve #1956.

@wesleytodd
Copy link

Glad to see this ended with a docs change. Not sure how much it matters, but even this wording is a bit incorrect in that it is "at the time the class is instantiated" right? For nearly all users I think that should be import time, but I almost feel like this could generate a few questions from folks if they are using the class and passing a cwd. Not sure I have a good suggestion seeing as even on the initial issue report my brain skipped right over the important detail of chdir in the middle of the program but just wanted to point it out.

My low effort idea is to phrase it as current working directory (default: process.cwd() at class instantiation time).

@rhettjay
Copy link
Member Author

I went back and forth on this locally before pushing up. I ended with import because of the PR on standard-engine. You're right though, it'll probably add to the confusion. I wonder if perhaps we opt for yours with a guided hint:
current working directory (default: process.cwd() at class instantiation time -- see standard-engine docs
Thoughts?

@wesleytodd
Copy link

Honestly I am not very opinionated, just wanted to share what caught my eye reading it. I like the wording in this last more explicit one, but I agree that if end users wanting to do this understand and use "at import time" more then we should go with that. @regseb any thoughts?

@regseb
Copy link

regseb commented Dec 19, 2023

Standard only exports an object with the lintText() and lintFiles() functions. Is there a Node.js API?

This is an instance of the StandardEngine class, but the user doesn't know it. He may think that the Standard code is:

export default {
    "lintText": function (text, opts) {
        // Do something.
        return result;
    },
    "lintFiles": function (files, opts) {
        // Do something.
        return result;
    },
};

I think that with the term "at class instantiation time", the user may be confused: "which class?"

I find the wording "at import time" correct.

@rhettjay
Copy link
Member Author

rhettjay commented Aug 1, 2024

Given the issues with governance I don't believe it's likely this will be merged. Closing the PR. Check out neo-standard as an alternative.

@rhettjay rhettjay closed this Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

standard.lintFiles() doesn't use process.cwd()
3 participants