Skip to content

Beyondevil/cleanup#28

Merged
BeyondEvil merged 2 commits intocombined-fe-and-befrom
beyondevil/cleanup
Mar 5, 2023
Merged

Beyondevil/cleanup#28
BeyondEvil merged 2 commits intocombined-fe-and-befrom
beyondevil/cleanup

Conversation

@BeyondEvil
Copy link
Copy Markdown
Owner

No description provided.

@BeyondEvil BeyondEvil force-pushed the beyondevil/cleanup branch 9 times, most recently from 676981e to bf424fa Compare February 11, 2023 20:36
class DataManager {
setManager(data) {
const collapsedCategories = getCollapsedCategory()
const collapsedCategories = [...getCollapsedCategory(), 'passed']
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

snyggt!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Tack! <3

Comment on lines +95 to +100
let durationText
if (formattedAccTime.hasOwnProperty('ms')) {
durationText = formattedAccTime.ms
} else {
durationText = formattedAccTime.seconds
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
let durationText
if (formattedAccTime.hasOwnProperty('ms')) {
durationText = formattedAccTime.ms
} else {
durationText = formattedAccTime.seconds
}
const durationText = formattedAccTime.ms ?? formattedAccTime.seconds

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I went with the ternary option, the ?? is super nice but I feel it's too much magic for your average pythonista to handle. 😅 @drRedflint

Copy link
Copy Markdown

@drRedflint drRedflint left a comment

Choose a reason for hiding this comment

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

Looking good!

@BeyondEvil BeyondEvil force-pushed the beyondevil/cleanup branch 17 times, most recently from 0893670 to 20365f7 Compare February 12, 2023 00:56
@BeyondEvil BeyondEvil force-pushed the beyondevil/cleanup branch 27 times, most recently from 6678136 to f5455fe Compare February 14, 2023 00:36
- "4444:4444"
- "7900:7900"
volumes:
- "${PWD}:/reports${PWD}:ro"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why ro instead of Z

Copy link
Copy Markdown
Owner Author

@BeyondEvil BeyondEvil Feb 15, 2023

Choose a reason for hiding this comment

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

Just experimenting, tried without ro as well.

Not sure what Z does.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

z ensures user id mapping and selinux relabeling

aka allow to read user owned files

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

it has to be capital z tho

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Oh, interesting! Maybe that's the key to solving this!

Will give it a try ASAP, thanks! 🙏

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

we jsut figured that the root casue of the permissions is
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/tmpdir.py#L154-L159

--basetmp has to go 😮‍💨

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@BeyondEvil BeyondEvil force-pushed the beyondevil/cleanup branch from f5455fe to ea010b9 Compare March 3, 2023 00:49
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

Successfully merging this pull request may close these issues.

3 participants