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

fix: Remove test reference to DEFRA_ROOTDIR env var #1328

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

AndrewSisley
Copy link
Contributor

Relevant issue(s)

Resolves #1327

Description

This environment variable does not exist, so I think we should remove all references to it.

@AndrewSisley AndrewSisley added documentation Improvements or additions to documentation action/no-benchmark Skips the action that runs the benchmark. area/cli Related to the CLI binary area/config Related to configuration labels Apr 11, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.5 milestone Apr 11, 2023
@AndrewSisley AndrewSisley requested a review from a team April 11, 2023 17:38
@AndrewSisley AndrewSisley self-assigned this Apr 11, 2023
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I1327-DEFRA_ROOTDIR branch from ba7411b to 8613d86 Compare April 11, 2023 17:42
@AndrewSisley
Copy link
Contributor Author

AndrewSisley commented Apr 11, 2023

Config file commit dropped, as it is already being removed in #1318

Will just drop the test here

@AndrewSisley AndrewSisley changed the title fix: Remove references to DEFRA_ROOTDIR env var fix: Remove test reference to DEFRA_ROOTDIR env var Apr 11, 2023
@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Merging #1328 (96e73fc) into develop (946c9d2) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1328      +/-   ##
===========================================
+ Coverage    70.60%   70.63%   +0.03%     
===========================================
  Files          184      184              
  Lines        17784    17784              
===========================================
+ Hits         12556    12562       +6     
+ Misses        4274     4270       -4     
+ Partials       954      952       -2     

see 3 files with indirect coverage changes

@AndrewSisley AndrewSisley force-pushed the sisley/fix/I1327-DEFRA_ROOTDIR branch from 38e4a6b to 8613d86 Compare April 11, 2023 18:06
Copy link
Contributor

@orpheuslummis orpheuslummis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Test does not add any value IMO, and can almost suggest that this env var exists and is meaningful
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I1327-DEFRA_ROOTDIR branch from 8613d86 to 96e73fc Compare April 11, 2023 19:05
@AndrewSisley AndrewSisley merged commit 8ea58ce into develop Apr 11, 2023
@AndrewSisley AndrewSisley deleted the sisley/fix/I1327-DEFRA_ROOTDIR branch April 11, 2023 19:27
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/cli Related to the CLI binary area/config Related to configuration documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review $DEFRA_ROOTDIR
2 participants