Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Fix/Refactor: missing siblingVersion from sibling check #829

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix/Refactor: missing siblingVersion from sibling check #829

wants to merge 4 commits into from

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented Jan 29, 2025 •
edited
Loading

Main changes:

Fixes #781

@lucas-zimerman lucas-zimerman changed the title refactor sibling check code Fix/Refactor: missing siblingVersion from sibling check Jan 29, 2025
@lucas-zimerman lucas-zimerman marked this pull request as ready for review January 29, 2025 18:36
@lucas-zimerman lucas-zimerman requested review from 0Calories and krystofwoldrich and removed request for 0Calories January 29, 2025 18:42
krystofwoldrich
krystofwoldrich reviewed Feb 4, 2025
View reviewed changes
scripts/check-siblings.js Outdated
Comment on lines 10 to 15
const IsE2E = env.sentry_e2e == 'true';

function LogE2E(message) {
IsE2E && console.log("E2E_TEST: " + message);
}

Copy link
Member

Choose a reason for hiding this comment

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

Let's isolate the fix and remove the E2E test changes from this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good to me, I have removed the e2e logs on e5640b0

krystofwoldrich
krystofwoldrich approved these changes Feb 6, 2025
View reviewed changes
Copy link
Member

@krystofwoldrich krystofwoldrich left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LG. I think the refactor and message should also be a separate change, but I checked the code and it looks good.

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@krystofwoldrich krystofwoldrich krystofwoldrich approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ReferenceError: siblingVersion is not defined while installing the package
2 participants
@lucas-zimerman @krystofwoldrich