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

Create update_editing-units.md #1005

Closed
wants to merge 1 commit into from

Conversation

1jc
Copy link

@1jc 1jc commented Sep 7, 2023

issue #906

Description

Describe the bug
If you change what a unit represents then bad things can happen. A conversion might now exist between different types of units. During creation this is not allowed. This is esp. dangerous if this is the meter graphing unit.

Expected behavior
OED should stop a user from making this type of edit and be told to fix up the dependencies. Similar types of checks happen in group edits.

Additional context
None

Fixes #[906]

Type of change

(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])

  • Note merging this changes the database configuration.
  • [X ] This change requires a documentation update

Checklist

(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)

  • [ X] I have followed the OED pull request ideas
  • I have removed text in ( ) from the issue request
  • [X ] You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

None

@1jc
Copy link
Author

1jc commented Sep 7, 2023

@huss please review

@huss
Copy link
Member

huss commented Sep 8, 2023

Thanks to @1jc for this pull request. From what I see this is new documentation. This type of information is found in the OED help pages. The documentation for the new meter admin pages are still under development and some of your text may be useful for that. I think there may have been some confusion about what the issue wanted addressed. The idea was to modify the code so it would be impossible for an admin to make this undesirable change. If that was the case then the documentation would describe why it is not allowed but it would never happen in practice. Given this, do you still want to pursue this issue? I am happy to discuss this with you to clarify if that would help. Again, we appreciate your efforts working on OED and are here to help. If I have misunderstood this work then please let me know.

@1jc
Copy link
Author

1jc commented Sep 8, 2023

@huss thank you for your response. as quoted " for this pull request. From what I see this is new documentation. The documentation for the new meter admin pages are still under development and some of your text may be useful for that. The idea was to modify the code so it would be impossible for an admin to make this undesirable change. If that was the case then the documentation would describe why it is not allowed but it would never happen in practice. Given this, do you still want to pursue this issue? " @huss yes I believe this new documentation would clarify the issue of an admin making an undesirable change - documentation would describe why it not allowed but it would never happen in practice. I believe this issue is resolved. Otherwise, can this new documentation be merged as new documentation? @1jc

@huss
Copy link
Member

huss commented Sep 8, 2023

Thanks to @1jc for your quick response and continued interest in OED. OED has the philosophy that we should make it as easy as possible for people to interact with the software. This includes supporting less knowledgeable people as admins so we work hard to not allow them to do things that are detrimental to the site. Thus, we do want to modify the software to stop these actions. Having said that, I am happy to see how much of your documentation can be added to the version 1.0.0 help pages that I am currently working on and I expect to be able to do that. Unfortunately, I cannot easily merge your documentation into the help web pages but I will note on this PR and on the help page commit that it includes work you contributed to. Is that okay with you?

@1jc
Copy link
Author

1jc commented Sep 9, 2023

@huss It is okay with me on your above statement @1jc

@1jc
Copy link
Author

1jc commented Sep 9, 2023

@1jc made comment on issue #906

@huss
Copy link
Member

huss commented Sep 16, 2023

This text was included as appropriate in the planned update to the meter creation page in this commit. Given this, this PR can now be closed. Again, thanks to @1jc for submitting this to OED.

@huss huss closed this Sep 16, 2023
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.

None yet

2 participants