Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

fixing minor issues rebased #4593

Merged
merged 4 commits into from
Jun 17, 2022
Merged

fixing minor issues rebased #4593

merged 4 commits into from
Jun 17, 2022

Conversation

Golovneva
Copy link
Contributor

@Golovneva Golovneva commented Jun 15, 2022

Patch description
Adding/fixing comments. Fixing main to make it pass tests.

Testing steps

Other information

@Golovneva Golovneva marked this pull request as ready for review June 15, 2022 16:23
@klshuster
Copy link
Contributor

We removed the tensorboard from requirements.txt yesterday, and I think this fixes the cleaninstall issues; thus, I'm not sure this PR is necessary anymore

@Golovneva
Copy link
Contributor Author

We removed the tensorboard from requirements.txt yesterday, and I think this fixes the cleaninstall issues; thus, I'm not sure this PR is necessary anymore

Not sure what do you mean. Main is failing. You can also look at previous runs in this commit thread on latest main, they are all failing. tensorboard removal introduced new breakages

@klshuster
Copy link
Contributor

i think those are spurious errors; it seems that #4590 had passing tests when it was landed

@Golovneva
Copy link
Contributor Author

I'm not sure how it passed, but both main and my commits where are failing systematically with the same error. I have re-runned them several times, always same error here and in #4589
There might be a change in some other library between commit was tested and merged again. Re-emerging error is "ModuleNotFoundError: No module named 'setuptools.command.build": https://app.circleci.com/pipelines/github/facebookresearch/ParlAI/11525/workflows/922186c8-dc2c-4e51-9ef7-aa700374c9f5/jobs/94066

@Golovneva
Copy link
Contributor Author

Will create a PR with text changes only

@Golovneva Golovneva closed this Jun 15, 2022
@Golovneva Golovneva reopened this Jun 16, 2022
@Golovneva Golovneva mentioned this pull request Jun 16, 2022
Copy link
Contributor

@klshuster klshuster left a comment

Choose a reason for hiding this comment

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

approving given everything passes. Thanks!

@Golovneva Golovneva merged commit 304fe1c into main Jun 17, 2022
@Golovneva Golovneva deleted the olggol/comments-2 branch June 17, 2022 17:25
@Golovneva
Copy link
Contributor Author

Huh, still failed in mainline. Is it possible that some other changes are required in circleci config?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants