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.

Sign up for GitHub

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

Jump to bottom

(chore) Remove created_by field #475

Merged
merged 1 commit into from
Mar 13, 2024
Merged

(chore) Remove created_by field #475

merged 1 commit into from
Mar 13, 2024

Conversation

anik120
Copy link
Contributor

@anik120 anik120 commented Mar 13, 2024 •
edited by lehors
Loading

Please see #657 for more details.

@anik120 anik120 requested a review from a team as a code owner March 13, 2024 16:21
@lehors
Copy link
Contributor

lehors commented Mar 13, 2024

You're missing the signoff.
We'll need to update the lint checker accordingly for this to pass the test. @ckadner

@lehors
Copy link
Contributor

lehors commented Mar 13, 2024

We need to either change the lint checker in a separate PR or override the failed result here.

@lehors lehors mentioned this pull request Mar 13, 2024
Closed
@anik120
Copy link
Contributor Author

anik120 commented Mar 13, 2024 •
edited
Loading

@lehors oops added the signoff.

Also I modified the linter here to stop checking for the created_by field, but looks like the linter is complaining about there being less than 5 seed examples for some files and empty task description for some files.

I'm guessing these checks were introduced after these files were introduced to the repo.

@anik120
Remove created_by field
0925901 
Please see https://github.com/instruct-lab/cli/issues/220
for more details.

Signed-off-by: Anik Bhattacharjee <[email protected]>
@anik120
Copy link
Contributor Author

anik120 commented Mar 13, 2024

Force pushed again after rebasing off of the latest commit to main

@lehors
Copy link
Contributor

lehors commented Mar 13, 2024

ok. I saw your PR was modifying the linter but thought the patch hadn't been applied before running the test and that's why it had failed. I guess we'll just have to override that failure for the time being. Maybe we can open an issue to fix the errors later on.

lehors
lehors approved these changes Mar 13, 2024
View reviewed changes
Copy link
Contributor

@lehors lehors left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@lehors lehors merged commit ba25a8a into instructlab:main Mar 13, 2024
1 of 2 checks passed
xukai92 added a commit that referenced this pull request Mar 13, 2024
@xukai92
Revert "(chore) Remove created_by field (#475)"
1348da3 
This reverts commit ba25a8a.
@xukai92 xukai92 mentioned this pull request Mar 13, 2024
Merged
xukai92 added a commit that referenced this pull request Mar 13, 2024
@xukai92
Revert "(chore) Remove created_by field (#475)"
01116d4 
This reverts commit ba25a8a.

Signed-off-by: Kai Xu <[email protected]>
lehors pushed a commit that referenced this pull request Mar 13, 2024
@xukai92
Revert "(chore) Remove created_by field" (#497)
d0d214f 
Reverts #475

Signed-off-by: Kai Xu <[email protected]>
@hickeyma hickeyma deleted the remove-created-by-field branch March 14, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lehors lehors lehors 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.

None yet
2 participants
@anik120 @lehors