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

corrections after refactoring #143

Closed
wants to merge 5 commits into from

Conversation

arnekaulfuss
Copy link

IMHO differentiation between project field and issue field is needed here.
These are my first steps with RoR and redmine plugins, so maybe it's an inelegant approach.
I haven't found any naming conventions or guidelines for redmine(-plugins ) so please correct my code if it's bad!

@@ -112,10 +113,17 @@ def receive_issue_reply_with_helpdesk(issue_id, from_journal=nil)
return last_journal
end

def custom_field_value(issue,name)
# differentiate between project field and issue field
def custom_field_value(issue,name,isproject_field = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

That is over engineering :)
CustomField_ID has sequential numbering. And each field assigned either to project or to issue.

Copy link
Author

Choose a reason for hiding this comment

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

Since it's not used elsewhere it's over engineered, you're right. And what I've learned by writing some ruby, it's not nice ruby style, too ;).
I just did not want to redo the cleaning work that was done before as I like easy to read code and it was already in an own function.

@jfqd
Copy link
Owner

jfqd commented May 11, 2020

Fixed.

@jfqd jfqd closed this May 11, 2020
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

3 participants