Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Fix NavBar item to Community Configurations #110

Merged
merged 2 commits into from
Aug 7, 2020
Merged

Conversation

sladyn98
Copy link
Contributor

@sladyn98 sladyn98 commented Aug 3, 2020

No description provided.

@sladyn98 sladyn98 requested a review from a team as a code owner August 3, 2020 12:22
@sladyn98 sladyn98 linked an issue Aug 3, 2020 that may be closed by this pull request
@sladyn98 sladyn98 requested a review from martinda August 3, 2020 15:20
@martinda
Copy link
Contributor

martinda commented Aug 3, 2020

The tests appear to fail during Windows testing. Different OSes have different line terminations (to the despair of programmers). Check your code for how the comparison is done and how the files are loaded. I see a call to Files.readAllBytes() which may not be the best way to read a file due to the line ending characters (EOL). You should be able to find a way to load files without having to worry about line endings.

@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 3, 2020

@martinda I searched on Google but cannot seem to find any other method to read. Is there something I am missing.should I just include or exclude new lines from the test files .

martinda
martinda previously approved these changes Aug 3, 2020
Copy link
Contributor

@martinda martinda left a comment

Choose a reason for hiding this comment

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

Once the tests are fixed, this change can go through. You could submit a separate PR to fix the tests. I do not know why the tests would pass before, I am not sure it's even worth investigating. At this point we just want to fix them.

@martinda martinda dismissed their stale review August 3, 2020 16:02

Ok, so I don't want this PR to be merged until the tests are passing. We don't want to corrupt the master branch.

@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 3, 2020

@martinda should I close and reopen that will kickstart the build again maybe it will pass since it was passing before

@sladyn98 sladyn98 closed this Aug 3, 2020
@sladyn98 sladyn98 reopened this Aug 3, 2020
@martinda
Copy link
Contributor

martinda commented Aug 3, 2020

I think there is an issue with calling Files.getAllBytes(). I think it returns a different EOL depending on the OS. Maybe it does not matter in this case? You can try to re-trigger the test of course.

@sladyn98
Copy link
Contributor Author

sladyn98 commented Aug 3, 2020

@martinda yeah I guess it does not matter since the previous builds were passing let's retrigger and check

@kwhetstone
Copy link
Contributor

I had thought that the tests at one point were only supposed to be run on Linux?

@sladyn98 sladyn98 merged commit 342dce3 into master Aug 7, 2020
sladyn98 pushed a commit to sladyn98/custom-distribution-service that referenced this pull request Aug 9, 2020
* Fix Navbar item

* Update Header.js
sladyn98 pushed a commit to sladyn98/custom-distribution-service that referenced this pull request Aug 9, 2020
* Fix Navbar item

* Update Header.js
@sladyn98 sladyn98 added the bugfix A PR that fixes a bug - used by Release Drafter label Aug 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix A PR that fixes a bug - used by Release Drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate menu item: Package Generation
3 participants