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

Change datalog CSV download mime type from text/plain to text/csv #339

Merged

Conversation

martinwork
Copy link
Collaborator

Relates to #327.

@microbit-matt-hillsdon
Copy link
Collaborator

As things stand I think this PR will have no effect. See this doc for how to convert header.html into a change to the C++ file that embeds it. I can do this if needed.

I'm a bit puzzled as to why I didn't do this in the first place so I'd like to re-run some testing to make sure it doesn't make anything worse. It saves me two bytes in the header HTML which were very hard to come by! I'll try to do that this week.

@martinwork
Copy link
Collaborator Author

Thanks @microbit-matt-hillsdon

@JohnVidler JohnVidler added this to the v0.2.58 milestone Jun 29, 2023
@microbit-carlos microbit-carlos removed this from the v0.2.58 milestone Jul 11, 2023
@microbit-carlos
Copy link
Collaborator

microbit-carlos commented Jul 11, 2023

I've pushed a commit updating the MicroBitLog.cpp file and it's tripled the available free space from 1 byte to 3 🎉 🤣
@microbit-matt-hillsdon let us know once you've had a chance to run the tests you had in mind and we can merge the PR 👍

@JohnVidler
Copy link
Collaborator

Looks good to me, lets get this merged in. 👍

@JohnVidler JohnVidler merged commit e88cb96 into lancaster-university:master Nov 13, 2023
@microbit-carlos
Copy link
Collaborator

@microbit-matt-hillsdon / @microbit-robert this PR has been merged to CODAL, it mostly sets the CSV mime type from text/plain to text/csv. Letting you know as I assume the same mime type should probably be applied to any other CSV downloads for iOS.

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