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

Added data types in Lua #692

Merged
merged 7 commits into from
Oct 29, 2023
Merged

Conversation

ayaan-qadri
Copy link
Contributor

@ayaan-qadri ayaan-qadri commented Oct 28, 2023

What GitHub issue does this PR apply to?

Resolves #580

What changed and why?

I added data type feature for Lua as per issue #580

(If editing Django app) Please add screenshots

Checklist

  • I claimed any associated issue(s) and they are not someone else's
  • I have looked at documentation to ensure I made any revisions correctly
  • I tested my changes locally to ensure they work
  • (If editing Django) I have added or edited any appropriate unit tests for my changes

Last year(Hacktoberfest2022) i worked for Lua language for codethesaur.us so i have already familiar with this project.

@ayaan-qadri
Copy link
Contributor Author

Please add 'hacktoberfest-accepted' label if you are satisfied with PR.

@geekygirlsarah geekygirlsarah temporarily deployed to codethesaurus-test-pr-692 October 28, 2023 13:29 Inactive
@geekygirlsarah
Copy link
Member

Hey, thanks for adding this!

First, can you leave a comment on #580 to claim it as yours? (You checked it on the PR but you didn't actually do it.) Sometimes multiple people try to work on an issue together and it sucks that someone doesn't know someone else is working on it.

Second, the whole project is opted in to Hacktoberfest, no label needed on the PR.

Third, multiple things like signed and unsigned 8-bit integers are "Undefined" because you didn't specify if they do or don't exist in the language. You should fill those in and if the language doesn't have them, mark them as "not-implemented". (See the documentation on why.) You can use the python manage.py generate_template command to generate a full template to fill in the gaps you're missing.

image

Copy link
Member

@geekygirlsarah geekygirlsarah left a comment

Choose a reason for hiding this comment

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

Missing concepts leaving "unknown" answers on the pages.

@ayaan-qadri
Copy link
Contributor Author

First of all sorry for not commenting on issue and checked it.

Second if this project opted into hacktoberfest still PR/MERGE need label look:

-Screenshot_2023-10-28-19-26-34-67_40deb401b9ffe8e1df2f1cc5ba480b12.jpg

Screenshot_2023-10-28-19-26-31-84_40deb401b9ffe8e1df2f1cc5ba480b12.jpg

third, i have done "not-implement":true still it shows unknown… idk why?

@geekygirlsarah
Copy link
Member

#692 isn't accepted because I haven't merged it in yet. When I do merge it, it should go into waiting and then be approved after 7 days. (This works for everyone so far, including me.)

image

https://github.com/thecuriousteam/pyliblog should automatically work because hacktoberfest is tagged on the project. (The other hacktoberfest tags don't matter. I add things like hacktoberfest2023 in case people search for them.) So
image

For the not-implemented part, some of the fields say "Not implemented in this langauge." Those are right. The fields that aren't are "unknown" because the fields are missing. Those are the fields that need to be added in.

Ex: signed 8-bit integers are "unknown" because

    "signed_integer_8_bit": {
      "name": "Signed 8-bit integer",
      "code": [
        ""
      ]
    },

is missing.

@ayaan-qadri
Copy link
Contributor Author

Okayy, I'll correct it soon.

@ayaan-qadri
Copy link
Contributor Author

btw @geekygirlsarah, this same thing is also happening in Python's data types.

@geekygirlsarah geekygirlsarah temporarily deployed to codethesaurus-test-pr-692 October 29, 2023 17:01 Inactive
Copy link
Member

@geekygirlsarah geekygirlsarah left a comment

Choose a reason for hiding this comment

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

Thanks for adding the other fields back in. I'll approve and merge! Happy Hacktoberfest!

@geekygirlsarah
Copy link
Member

btw @geekygirlsarah, this same thing is also happening in Python's data types.

Yes, Python, C++, and Java data types were the first 3 things added to CT back in 2017 or so. The data types concept has evolved since then and no one's gone back to update them. There's a few other holes in thesauruses and just merged in a feature to point out they could be contributed to.

@geekygirlsarah geekygirlsarah merged commit 7ae0daa into codethesaurus:main Oct 29, 2023
4 checks passed
@ayaan-qadri
Copy link
Contributor Author

btw @geekygirlsarah, this same thing is also happening in Python's data types.

Yes, Python, C++, and Java data types were the first 3 things added to CT back in 2017 or so. The data types concept has evolved since then and no one's gone back to update them. There's a few other holes in thesauruses and just merged in a feature to point out they could be contributed to.

I see, I'm planning to close all issue related to Lua then i would like resolve it.

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.

[Lua] Add data types
2 participants