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

Use dynamic OS-detected path separator in place of literal OS-specific separator #103

Closed
armanrahman22 opened this issue Jul 19, 2021 · 0 comments · Fixed by #105
Closed
Labels
enhancement New feature or request

Comments

@armanrahman22
Copy link
Contributor

Link to original issue on forked repo: inclusive-dev-tools#11

Problem Statement

In the C:\_wrk\inclusive-dev-tools\woke\pkg\ignore\ignore_test.go file, run-time detection of the host OS is used to permit the tests to run/pass cross-platform as shown in the following excerpt:

	assert.False(t, i.Match("not/foo"))
	assert.True(t, i.Match("my/files/file1"))
	assert.False(t, i.Match("my/files"))

	if runtime.GOOS == "windows" {
		assert.False(t, i.Match(`not\foo`))
		assert.True(t, i.Match(`my\files\file1`))
		assert.False(t, i.Match(`my\files`))
	}

Long-term maintainability of the test could be improved by expressing the test paths not as string-literals differing only by their path-separator character but instead leveraging os.PathSeparator (from the os package) and filepath (from the path/filepath package). See https://stackoverflow.com/questions/9371031/how-do-i-create-crossplatform-file-paths-in-go for guidance.

Acceptance Criteria

  • ignore_test.go updated to use the dynamically-interpreted path-separator character in lieu of explicit "/" or "\"
@armanrahman22 armanrahman22 added the enhancement New feature or request label Jul 19, 2021
@caitlinelfring caitlinelfring linked a pull request Jul 20, 2021 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant