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.

Sign up for GitHub

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

Jump to bottom

Checksync uses current working directory for marker location rather than the passed location or the config file #1262

Closed
somewhatabstract opened this issue May 7, 2023 · 1 comment · Fixed by #1305
Closed

Checksync uses current working directory for marker location rather than the passed location or the config file #1262

somewhatabstract opened this issue May 7, 2023 · 1 comment · Fixed by #1305
Assignees
@somewhatabstract
Labels
bug Something isn't working good first issue Good for newcomers
Milestone
Checksync 5.0.0

Comments

@somewhatabstract
Copy link
Owner

When running checksync from a location, it uses the current working directory for its discovery; but if a config file is given, or a location is passed, it should use that rather than the current working directory.

Otherwise, it's impossible to use outside of the intended directory, which is counter-intuitive.
@somewhatabstract somewhatabstract added the bug Something isn't working label May 7, 2023
@somewhatabstract somewhatabstract added this to the Checksync 5.0.0 milestone May 7, 2023
@somewhatabstract somewhatabstract self-assigned this May 7, 2023
@somewhatabstract
Copy link
Owner Author

Thinking on this. I think that the suggested approaches are wrong for this. Instead, we should support a separate --cwd argument for specifying the working directory and a corresponding configuration file field. That way it is possible to override the current working directory for a specific invocation - the default would remain the folder that it is executed in.

@somewhatabstract somewhatabstract added the good first issue Good for newcomers label May 14, 2023
@somewhatabstract somewhatabstract mentioned this issue Jun 11, 2023
Merged
somewhatabstract added a commit that referenced this issue Jun 11, 2023
@somewhatabstract
Add cwd arg and cwd behavior (#1305)
14bbc5f 
## Summary:
This adds a new `--cwd` argument to override the current working directory that checksync uses.

More importantly, it now sets the current working directory to the location of the configuration file, if one is loaded. This ensures that paths/globs specified in a configuration file are interpreted relative to the configuration file, not the current working directory, which makes it a lot easier to reason about the configuration file definition.

Issue: fixes #1262

## Test plan:
`yarn test`

Author: somewhatabstract

Reviewers: jeremywiebe, kevinbarabash

Required Reviewers:

Approved By:

Checks: ✅ CodeQL, ✅ codecov/project, ✅ Test and build (macOS-latest, 18.x), ✅ Test and build (macOS-latest, 16.x), ✅ Test and build (ubuntu-latest, 18.x), ✅ Test and build (ubuntu-latest, 16.x), ✅ Test and build (windows-latest, 18.x), ✅ Test and build (windows-latest, 16.x), ✅ Integration tests (windows-latest, 18.x), ✅ Integration tests (windows-latest, 16.x), ✅ Integration tests (macOS-latest, 18.x), ✅ Integration tests (macOS-latest, 16.x), ✅ Integration tests (ubuntu-latest, 18.x), ✅ Integration tests (ubuntu-latest, 16.x), ✅ Update test coverage (ubuntu-latest, 16.x), ✅ Lint and static types check (ubuntu-latest, 16.x), ✅ Analyze (javascript), ⏭  dependabot

Pull Request URL: #1305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@somewhatabstract somewhatabstract

Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Milestone
Checksync 5.0.0
Development

Successfully merging a pull request may close this issue.

Add cwd arg and cwd behavior somewhatabstract/checksync
1 participant
@somewhatabstract