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.

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

Feature: add the ability for a user to amend a commit #24

Merged
merged 5 commits into from
Jun 26, 2024

Conversation

nerdalert
Copy link
Member

@nerdalert nerdalert commented Jun 22, 2024

Details in #15

Closes #15
Closes #29

Details in instructlab#15

Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
@nerdalert nerdalert marked this pull request as draft June 22, 2024 18:12
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
- Cleaned up submissions to better reflect the taxonomy schemas.
- Changed task_details to submission_summary to be used for the
commit title. Add a character limit to that summary field to adhere
to commit title length best practice.
- Fixed the dco format.
- Set revision field in the skill attribution to '-'

Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
@nerdalert nerdalert force-pushed the feat-edit-prs branch 2 times, most recently from 1654e9b to 0f63f77 Compare June 24, 2024 06:09
@nerdalert nerdalert marked this pull request as ready for review June 24, 2024 13:56
@nerdalert
Copy link
Member Author

nerdalert commented Jun 24, 2024

View the code running here for testing. Please try it if you have the time. The test repo CI will label the PR and it will populate in the dashboard once you submit it. You can from there edit it. http://34.229.17.203:4000/

Here is a populated dashboard:
image

Modal popup:
image

Edit Submission:
image

- Also adds a pop modal for viewing the current yaml config
- Replace the upstream repo .envs with public nextjs variables
that are prefixed with `NEXT_PUBLIC_`

Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
.env.example Outdated
@@ -12,3 +12,5 @@ GITHUB_TOKEN=<TOKEN FOR OAUTH INSTRUCTLAB MEMBER LOOKUP>
TAXONOMY_REPO_OWNER=<GITHUB_ACCOUNT>
TAXONOMY_REPO=<REPO_NAME>

Choose a reason for hiding this comment

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

Are TAXONOMY_REPO_OWNER and TAXONOMY_REPO still in use after this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

@toraponibm thanks for catching that. I had to switch the ENVs for be prefixed with NEXT_PUBLIC_ for them to be accessible by the client side in NextJs.

Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Copy link

@toraponibm toraponibm left a comment

Choose a reason for hiding this comment

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

LGTM

@juancappi
Copy link

I tested it to work on the CI environment. I do see a couple usability issues. Not sure if I should report them or it's assumed that it's a WIP and things are not polished yet.

Given the scope is purely about amend, this LGTM.

@nerdalert
Copy link
Member Author

I tested it to work on the CI environment. I do see a couple usability issues. Not sure if I should report them or it's assumed that it's a WIP and things are not polished yet.

Given the scope is purely about amend, this LGTM.

@juancappi looking to merge it. Throw them at me. We can do them on follow up smaller PRs. Not a huge fan of big PRs like this but since it's a feature it sprawled on me. Happy to open the issues, ty for spinning it up!

Copy link
Member

@vishnoianil vishnoianil left a comment

Choose a reason for hiding this comment

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

LGTM

@vishnoianil vishnoianil merged commit 02d3358 into instructlab:main Jun 26, 2024
5 checks passed
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.

Add schema version to submissions Add the ability to amend a user submission
4 participants