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

Test/fixexport #1074

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Test/fixexport #1074

wants to merge 2 commits into from

Conversation

vmw
Copy link
Contributor

@vmw vmw commented Feb 13, 2020

This PR fixes two issues with exporting project notes;

  1. It fixes how accented charactors are exported, and
  2. It ensures that project remarks are properly exported.

@dromer
Copy link
Contributor

dromer commented Feb 15, 2020

Why is the alert about the cron-jobs commented out?

@christianlupus
Copy link
Collaborator

Also please rebase onto master to allow the checks to run.

This commit fixes the exporter adding accented characters to the file.
@vmw
Copy link
Contributor Author

vmw commented Sep 7, 2022

We've just rebased against master, and it seems to have been applied cleanly.

@vmw
Copy link
Contributor Author

vmw commented May 30, 2023

Is it possible to re-examine merging this into master? It presently looks like all the checks are passing.

@dromer
Copy link
Contributor

dromer commented May 30, 2023

@vmw can you un-comment the cronjob alert please? I'm ok with merging it after that.

This commit keeps the project remark connected to
the newly selected items in a project report.
@vmw
Copy link
Contributor Author

vmw commented May 30, 2023

I just forced pushed an update to this branch that ensures the warning is re-enabled;

667c17a

@dromer
Copy link
Contributor

dromer commented May 30, 2023

Hmm, slight style issue: you've introduced several lines with white-space.

Please try to keep the code clean.

@vmw
Copy link
Contributor Author

vmw commented May 30, 2023

Hmm, slight style issue: you've introduced several lines with white-space.
Please try to keep the code clean.

You're likely right - I think there previously was a thread about having an offline linter.

I'm definitely happy to apply a linter if there is one, but, as a bit of a exception, it's difficult to automate this for us.

The commit itself fixes several bugs - we're looking to update to master, to determine the feasibility of our dedicating an engineer to help us make some more substantial changes.

Allowing the merging of these commits would help us determine the work associated with doing this from master - which is why I'm digging up these old requests.

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.

4 participants